WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 20 months ago

Last modified 20 months ago

#21440 closed defect (bug) (fixed)

Twenty Twelve: verify IE7, IE8, and IE9 support

Reported by: obenland Owned by: lancewillett
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

As the title suggests.

Probably just a few layout tweaks needed.

Attachments (9)

21440.matchMedia.patch (1.1 KB) - added by SergeyBiryukov 21 months ago.
21440.ie7.png (19.5 KB) - added by SergeyBiryukov 21 months ago.
21440.image-unit-test.diff (663 bytes) - added by obenland 21 months ago.
21440.patch (1.6 KB) - added by sennza 21 months ago.
css3-mediaqueries.js (15.6 KB) - added by philiparthurmoore 21 months ago.
add-media-querie-support-1.diff (1.0 KB) - added by philiparthurmoore 21 months ago.
21440.2.patch (1.5 KB) - added by SergeyBiryukov 21 months ago.
21440.3.patch (1.3 KB) - added by obenland 21 months ago.
21440.image-width.diff (867 bytes) - added by obenland 21 months ago.
Makes Image Test, page 3, in Theme Unit Test valid

Download all attachments as: .zip

Change History (31)

comment:2 SergeyBiryukov21 months ago

  • Milestone changed from Awaiting Review to 3.5

21440.matchMedia.patch is an attempt to fix the JS error due to a missing window.matchMedia() method. An alternative would be to add something like matchMedia.js.

SergeyBiryukov21 months ago

comment:3 SergeyBiryukov21 months ago

The layout in IE 7 and IE 8 is almost the same (width is shifted to 100% as described in #20198), the only difference is that main menu is broken in IE 7 (21440.ie7.png).

comment:4 obenland21 months ago

It's probably the media queries that are causing this.

An easy way out could be adding another script to the <!--[if lt IE 9]> conditional, like respond.js.

comment:5 follow-up: obenland21 months ago

21440.image-unit-test.diff doesn't help with IE8 styling, but makes the IE8 hack address IE8 only. Fixes the 3rd Image Test in Theme Unit Test for all browsers.
Needs testing in IE8!

comment:6 in reply to: ↑ 5 SergeyBiryukov21 months ago

Replying to obenland:

but makes the IE8 hack address IE8 only.

Per @iandstewart, avoiding targeting just IE8 was intentional for Twenty Eleven (ticket:18775:11).

comment:7 lancewillett21 months ago

  • Summary changed from Twenty Twelve: Add IE8 support to Twenty Twelve: verify IE7, IE8, and IE9 support

comment:8 lancewillett21 months ago

Talking it over with drewstrojny: For IE7 we could just show mobile view, it doesn't need to have media query support.

comment:9 philiparthurmoore21 months ago

Input/Submit buttons on search widget and password protected fields need margin/height work in IE9 and down.

http://p-am.net/grabs/chrome-ie-input-submit-20120805-133521.png

comment:10 sennza21 months ago

  • Cc bronson@… added

sennza21 months ago

comment:11 follow-up: sennza21 months ago

As the widths and floats are declared in a media query IE7 and IE8 didn't haven't the correct layouts. Normally on clients sites I would address this with a JavaScript polyfill however I figured it would be better to address this with classes for Internet Explorer similar to how Twenty Eleven does and I added some IE7 and IE8 CSS conditionals to fix the layout.

I'm open to any suggestions of a better way to do this though :)

comment:12 in reply to: ↑ 11 philiparthurmoore21 months ago

Replying to sennza:

As the widths and floats are declared in a media query IE7 and IE8 didn't haven't the correct layouts. Normally on clients sites I would address this with a JavaScript polyfill however I figured it would be better to address this with classes for Internet Explorer similar to how Twenty Eleven does and I added some IE7 and IE8 CSS conditionals to fix the layout.

I'm open to any suggestions of a better way to do this though :)

Argh. I didn't realize you uploaded a patch for this before submitting #21489. Very sorry about that.

comment:13 philiparthurmoore21 months ago

Two solutions seemed to be proposed here: 1) class-based adjustments (see @sennza's patch) and 2) media queries for IE7/IE8 (see latest two patches). My preference is for media queries here but would love to here other thoughts on this.

Version 0, edited 21 months ago by philiparthurmoore (next)

SergeyBiryukov21 months ago

comment:14 SergeyBiryukov21 months ago

With 21440.patch, the layout looks proper in both IE 7 and IE 8 (the menu is still broken in IE 7 though, see 21440.ie7.png).

21440.2.patch adds body background color as well (and fixes tabs vs. spaces).

comment:15 follow-ups: lancewillett21 months ago

From chellycat in #21486

Removing width: auto from images, which was causing images resized in the editor to appear full width. The "width: auto" is an IE8 specific fix and should be moved to IE8-specific styles.

See patch on that ticket, would require adding the html element class switch to add .ie8 hooks.

comment:16 in reply to: ↑ 15 obenland21 months ago

Replying to lancewillett:

From chellycat in #21486

Removing width: auto from images, which was causing images resized in the editor to appear full width. The "width: auto" is an IE8 specific fix and should be moved to IE8-specific styles.

See patch on that ticket, would require adding the html element class switch to add .ie8 hooks.

21440.image-unit-test.diff doesn't. Would be more of a hack though.

comment:17 lancewillett21 months ago

In [21475]:

Twenty Twelve: add polyfill JS for matchMedia rules, avoids JS error in older IE versions. See #21440.

obenland21 months ago

comment:18 lancewillett21 months ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In [21482]:

Twenty Twelve: add basic styles for IE7 and IE8, supports basic layout now with the small-nav styles. Props obenland, closes #21440.

comment:19 in reply to: ↑ 15 obenland21 months ago

  • Keywords needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to lancewillett:

From chellycat in #21486

Removing width: auto from images, which was causing images resized in the editor to appear full width. The "width: auto" is an IE8 specific fix and should be moved to IE8-specific styles.

Not addressed yet. With the conditional .ie class in place it's a piece of cake.

obenland21 months ago

Makes Image Test, page 3, in Theme Unit Test valid

comment:20 obenland21 months ago

  • Keywords has-patch added

21440.image-width.diff Also makes Image Test, page 3, in Theme Unit Test valid

comment:21 lancewillett20 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [21516]:

Twenty Twelve: move IE8 styles to specific IE selector, props obenland. Closes #21440.

comment:22 Japh20 months ago

  • Cc japh@… added

@lancewillett, shouldn't there be some props for @sennza there too? I see his patch there and remember discussion about it at the Dev Hack Day after WCSF.

Note: See TracTickets for help on using tickets.