WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#25144 closed defect (bug) (fixed)

Twenty Fourteen: Fixed Header doesn't work in IE

Reported by: celloexpressions Owned by:
Milestone: 3.8 Priority: low
Severity: normal Version: 3.8
Component: Bundled Theme Keywords:
Focuses: Cc:

Description

The fixed header doesn't work at all in any version of IE. Same issue as #23841, probably a similar solution.

There are three ways we could fix this:

  • Remove the custom header image and do the fixed header with CSS only (see also #25094)
  • Use a CSS solution by default and only use the (lengthened/fixed) JS if the header image feature is being used (not default) - would also help performance of the fixed header for the majority of installs that probably won't use the header image
  • Add the additional, reasonably complicated JS needed to make this work and always use that even if there is no header image

Attachments (2)

25144.diff (1.9 KB) - added by celloexpressions 7 years ago.
switch to a css-only method for fixing the header and drop the masthead-fixed class
25144.1.diff (29.8 KB) - added by obenland 6 years ago.

Download all attachments as: .zip

Change History (14)

#1 @obenland
7 years ago

Related: #25094

@celloexpressions
7 years ago

switch to a css-only method for fixing the header and drop the masthead-fixed class

#2 follow-up: @celloexpressions
7 years ago

  • Keywords has-patch needs-testing added

25144.diff does the fixed header with CSS instead of JS, since the decision has been made to remove custom header support in #25094. This seems to work well cross-browser but should be tested thoroughly.

The header is not sticky at viewports below 770px, as before. If we bump that number up 12px or so we won't need to account for MP6's 46px-tall mobile admin bar.

#3 @lancewillett
6 years ago

  • Milestone changed from Awaiting Review to 3.8

Discussing in today's office hours. Next step is to try a JS solution for when a header image exists, otherwise pure CSS.

Last edited 6 years ago by lancewillett (previous) (diff)

#4 @celloexpressions
6 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

Current status is that we need to re-write the js handling, then only load that is there's a header image. If there's no header image we shouldn't run any js, and the existing patch's approach works. Otherwise, we need to load something like we did in #23841 (potentially in its own file, rather than in theme.js, so it's easier to conditionally load).

(this is needed because we decided to keep the header image feature)

#5 @iamtakashi
6 years ago

  • Cc takashi@… added

@obenland
6 years ago

#6 @lancewillett
6 years ago

  • Keywords needs-refresh added; needs-patch removed
  • Priority changed from normal to low

#7 follow-up: @celloexpressions
6 years ago

25144.diff does a lot more than the fixed header adjustments, although that part looks like it'll work...

Also, as mentioned above, we should consider changing the 770px breakpoint for fixing the header to align with MP6's 782px breakpoint where they make the admin bar super thick. Otherwise we'll need to add extra rules for the 12px in between.

#8 in reply to: ↑ 7 @iamtakashi
6 years ago

Replying to celloexpressions:

25144.diff does a lot more than the fixed header adjustments, although that part looks like it'll work...

Also, as mentioned above, we should consider changing the 770px breakpoint for fixing the header to align with MP6's 782px breakpoint where they make the admin bar super thick.

I'm fine with changing the breakpoint to 782px.

#9 in reply to: ↑ 2 @iamtakashi
6 years ago

Replying to celloexpressions:

The header is not sticky at viewports below 770px, as before. If we bump that number up 12px or so we won't need to account for MP6's 46px-tall mobile admin bar.

Just to note that in r25864 the break point for the nav has been changed to 782px.

#10 follow-up: @celloexpressions
6 years ago

Now that we have a css-only approach when there's no header image, the fixed header works fine in IE (tested on 11, but probably okay for all).

If we want to make it work with a custom header image, we have more work to do here. Otherwise we can close. I think we'll rarely see a need for the header now that we have a featured content slider, unless users have trouble using everything (ie, the header controls are too prominent and usable in relation to the customizer; core problem).

#11 in reply to: ↑ 10 @iamtakashi
6 years ago

Replying to celloexpressions:

Now that we have a css-only approach when there's no header image, the fixed header works fine in IE (tested on 11, but probably okay for all).

If we want to make it work with a custom header image, we have more work to do here. Otherwise we can close.

I'm ok with not serving the fixed header for IE users when a custom header is present.

#12 @lancewillett
6 years ago

  • Keywords needs-refresh removed
  • Resolution set to fixed
  • Status changed from new to closed

Agreed, I'm also OK with a non-fixed header for IE users with a custom header image.

Note: See TracTickets for help on using tickets.