Twenty Fourteen: Sticky menu problem

[SergeyKovalets]

I've noticed for a while now that in Firefox, the top menu doesn’t stick to the top of the screen when scrolling, but it still works in Chrome

[abcd95]

Hi @SergeyKovalets, Thank you for reporting this issue.

I've tested this functionality on my end and found that the menu correctly sticks to the top of the screen when scrolling in both Chrome and Firefox.

Could you please share your Firefox version, WordPress configuration, and more detailed reproduction steps?

[SergeyKovalets]

Hi @abcd95. Give me some time to give a precise answer

[SergeyKovalets]

I can reproduce this on test websites when I add an additional arbitrary link to a menu that already contains several pages. For example, a link to the homepage in the form of /.
The latest version of Firefox at the moment.

[SergeyKovalets]

If there is one added page in the menu and one custom link, the issue appears immediately. If there are only two custom links, the issue also appears.

[abcd95]

@SergeyKovalets I was still not able to reproduce even after following the exact steps mentioned by you -

Here is the screen recording which shows the menu and the different browser interactions -
​https://bvvrdzu01i.ufs.sh/f/vtiKpIr2gd0cElVmht51fXKixrg4AMtuw8DTvsoB3hEU9YCF

Let me know if there is something more to be done here.

[sabernhardt]

add an additional arbitrary link to a menu that already contains several pages.

Are these menu items wide enough to wrap to a second line? If yes, the header becomes more than 48 pixels tall. Twenty Fourteen adds a condition to ​'unfix the header' when it is taller.

[SergeyKovalets]

Thanks for getting back to me @abcd95 . It's strange behavior—when I use a custom link, it doesn't always work, but it does work 100% of the time if I just use a slash in the link field

​https://dropmefiles.net/en/P8wfUH5

[SergeyKovalets]

@sabernhardt No, the menu can have just a few links, but if even one of them has only a slash (/) in the link field, the menu stops sticking.

[SirLouen]

Reproduction Report

Description

❌ This report can't validate that issue can be reproduced.

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.4.6
• Server: nginx/1.27.4
• Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.4.6)
• Browser: Chrome 135.0.0.0
• OS: Windows 10/11
• Theme: Twenty Fourteen 4.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. Create a New Menu with Top Primary Menu location
2. Add a custom link with URL / according to this
3. Add 4 extra links ​https://i.imgur.com/m0MpI0u.png
4. Check if the sticky top bar scrolls
5. ❌ Error can't be reproduced, check video below

Additional Notes

I have tested both in trunk and in 6.7.2 and I can't reproduce in neither versions.

@SergeyKovalets following your exact steps I can't reproduce this either, exactly like @abcd95. Please check the video and images attached, and provide with additional information, something different to test. Also, make sure you are testing on a completely brand-new WordPress site to confirm it's not an issue made by any mods, plugins or whatever, introduced over time.

Supplemental Artifacts

Demo Video: ​https://streamable.com/pq1i2c

[sabernhardt]

Russian deactivates Twenty Fourteen's Lato font stylesheet (​as you know 😉).

Then Firefox somehow makes the #primary-menu 48.5 pixels tall when it contains a link to the current page. The header height remains 48 pixels with a font-weight of 900 in Lato, but it is 48.5 pixels with Arial as a fallback sans-serif.

One possible change you could make for your site is

.site-navigation .current_page_item > a,
.site-navigation .current_page_ancestor > a,
.site-navigation .current-menu-item > a,
.site-navigation .current-menu-ancestor > a {
  font-weight: 700;
}

However, I do not yet have a recommendation how to change the theme for everybody.

[SergeyKovalets]

Thanks so much for checking this @sabernhardt 🙂
I had a hunch that might be the problem, as the language being used was the only difference in the tests the others ran earlier. I just didn't get a chance to check it myself today.
So, it seems this affects anyone who has turned off the Lato font, since it doesn't support Cyrillic.

[sabernhardt]

I also considered setting a height on the menu or decreasing the line-height on each li, but I do not think either of those CSS options would be appropriate for all sites.

Maybe editing the script instead could be better:

if ( mastheadHeight >= 49 ) {
	body.removeClass( 'masthead-fixed' );
}

The else condition of the header image part of the script removes the masthead-fixed when the header height is greater than or equal to 49 pixels, and replacing 48 a few lines earlier would make them consistent. (We probably should also review the 781 width because it does not match 783 in the CSS, but not necessarily on this ticket.)

Setup steps:

1. Activate Twenty Fourteen.
2. Install one of these languages: Armenian, Azerbaijani, Belarusian, Bengali (Bangladesh), Bosnian, Bulgarian, Czech, Esperanto, Georgian, Greek, Hebrew, Khmer, Mongolian, Myanmar (Burmese), Russian, Serbian, Sinhala, Slovenian, Tamil (Sri Lanka), Telugu, Turkish, or Ukrainian.
3. Go to the Updates page and update translations.
4. Go to the Menus screen, and add items to the primary menu.
5. Visit one of the pages included in the primary menu.

[sabernhardt]

edits script condition to check if header height is 49 pixels or more

[SirLouen]

Reproduction Report

Description

❌ This report can't still validate that the issue can be reproduced with the new provided instructions. I provide a video to demo this on a new clean environment

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 135.0.0.0
• OS: Windows 10/11
• Theme: Twenty Fourteen 4.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Bug Reproduction Steps

(Using @sabernhardt steps)

1. Activate Twenty Fourteen.
2. Install Russian
3. Go to the Updates page and update the translation.
4. Go to the Menus screen, and add items to the primary menu.
5. Visit one of the pages included in the primary menu.
6. The primary menu scrolls correctly

Actual Results

1. ❌ Error condition doesn't occur (The main menu scrolls correctly)

Additional Notes

@sabernhardt more testing info required to see how it's possible to reproduce this consistently.

Supplemental Artifacts

Demo Screencast: ​https://streamable.com/zitj9o

[sabernhardt]

This might only happen with

• Firefox on Windows (11 Home 24H2),
• both Arial and Arial Black installed on the device (but not Lato), and
• Arial as the default sans-serif font for the menu links' language (character set) in browser settings.

Firefox adds a little extra height to the menu ul when the current menu item link uses Arial Black to meet the 900 font-weight.

[SirLouen]

• Firefox on Windows (11)

• both Arial and Arial Black installed on the device, and

• Arial as the default sans-serif font for the language (character set) in browser settings.

❌ I can't reproduce this

Screencast: ​https://streamable.com/t38r5g

[sabernhardt]

Russian menu, with current menu item in Arial Black, is 48.5 pixels tall

[sabernhardt]

I edited comment:16 because

1. the system should not have Lato installed, and
2. the menu items should be in the language that does not use Lato and falls back to Arial (Black).

These are the Russian/Cyrillic links in the image:
Главная
Биография
Фото
Вопрос\Ответ
Поддержка
Контакты

[SirLouen]

Test Report

Description

🟠 This report validates that the indicated patch works as expected with some caveats

Patch tested: https://core.trac.wordpress.org/attachment/ticket/63275/63275.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 135.0.0.0
• OS: Windows 10/11
• Theme: Twenty Fourteen 4.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Testing Instructions

1. Set up manually force the CSS masthead element ID to anything height over 48
2. 🐞 The top menu header will not scroll.
3. The reasons of why that height is over 48 are not many, but are. Don't try to reproduce this, moon, sun and Jupyter must be aligned the day you try it. Just trust.

Actual Results

1. 🟠 The Patch solves the problem if the height is just under or equal 49px. Still too precise for my pov.

Additional Notes

@sabernhardt why not mastehead >= 50 or even 60? I think that, as-is, it's so prone to end with the scroll broken for whatever reason. And I think that setting it to a bigger size, will not do any harm. I believe that the sole idea here is to avoid sticking the menu if it happens to become double sized (two lines because of the menu items, for example). So I would rather choose a much wider range (60px for example would do the trick).

[sabernhardt]

why not masthead >= 50 or even 60?

If the header is fixed at the top and anything more than 48 pixels tall, it will start to cover the page's content (#25554). One extra pixel is probably safe, maybe even two. However, the header would already start obscuring the top of a Tagline at 53 pixels tall, and other elements could be closer. Besides, the odd number 49 is already in the script. Trying to solve an uncommon situation in an old theme hopefully would not involve much change.

If the general idea of 63275.diff is good, the fixed header comment in the script likely would need updating too.

[SirLouen]

Replying to sabernhardt:

why not masthead >= 50 or even 60?

If the header is fixed at the top and anything more than 48 pixels tall, it will start to cover the page's content (#25554). One extra pixel is probably safe, maybe even two. However, the header would already start obscuring the top of a Tagline at 53 pixels tall, and other elements could be closer. Besides, the odd number 49 is already in the script. Trying to solve an uncommon situation in an old theme hopefully would not involve much change.

If the general idea of 63275.diff is good, the fixed header comment in the script likely would need updating too.

Not sure what you mean with "obscuring". Setting 60 or even 70 won't affect anything apart from the fact that if the masthead has more than height it will not scroll anymore (when it jumps into two lines or three lines, basically)

Edit: Ok, now I've seen the issue, I was looking into the main content, but it's in the sidebar. It's so extremely adjusted, like you say, that anything beyond those 53px is going to hinder the visibility. So I think its safe to stick to 50px for "browser-looseness" and call it a day.

PS: Why have you removed dev-feedback tag? Although you are in the core team, you told me you are not core committing. The problem is that "Future Release" without dev-feedback means "Ticket Orphaned" like 99% of the time (and even with dev-feedback 80% of the times).
Basically, I always put dev-feedback to all reports that have been tested and are ready to ship to bring attention from someone that can actually set a specific Milestone (like 6.8.1 or at least 6.9 in this case) or directly commit into trunk.
I'm trying to figure out the best way to move forward tickets that are 100% done and just need a deployment, thus avoiding increasing indefinitely the massive ball of 100% finished tickets that were never committed because they were orphaned. Maybe I should raise this topic in one of the next core-dev chats.
I believe I will raise this issue in the next dev-chat to see if I find some consensus on this topic.

[sabernhardt]

This is not 100% done. I question whether my own patch goes too far, which means the ticket should have 2nd-opinion.