Closed Bug 1589089 Opened 5 years ago Closed 5 years ago

Menulists (esp. bookmarks) are displayed horizonally

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- verified

People

(Reporter: itiel_yn8, Assigned: surkov)

References

(Regression)

Details

(Keywords: regression, reproducible)

Attachments

(2 files)

Attached image Screenshot (deleted) —

Using Windows 10, latest Nightly.

Basically, when you open a menulist, usually if the list is not too long, it will be displayed horizonally instead of vertically. And not always.
If the list is too long, then for a split second it'll be displayed horizonally and then revert back to being displayed vertically.

This happens in the following UIs:

  1. On the Bookmarks Toolbar, if you have a folder there and you click it
  2. On the menu bar > Bookmarks or History

This happens on 2 different machines, both are not sharing the same addons so we can probably rule this out.
Nothing appears on the console when this happens.

I'm not sure what is the STR yet, thus unable to check regression range atm. This started about a week or two ago.

See attached.

Note that this doesn't reproduce 100% of the time.

Probably something with the XBL removal/conversion? Brian, can someone investigate this?

Flags: needinfo?(bgrinstead)

Alex said he'd take a look.

Flags: needinfo?(bgrinstead) → needinfo?(surkov.alexander)

I recall seeing this in the wild once or twice, I even thought this is a new feature in Firefox design, it looked somewhat cool :) but it seems I cannot reproduce any longer. Any chance to double check, in case if something have magically fixed the issue over last week? Or any luck to come with reliable steps to reproduce?

Flags: needinfo?(surkov.alexander) → needinfo?(itiel_yn8)

(In reply to alexander :surkov (:asurkov) from comment #4)

I recall seeing this in the wild once or twice, I even thought this is a new feature in Firefox design, it looked somewhat cool :) but it seems I cannot reproduce any longer. Any chance to double check, in case if something have magically fixed the issue over last week? Or any luck to come with reliable steps to reproduce?

Sorry, I can still reproduce it on the latest Nightly :-(
Still no STR.

Flags: needinfo?(itiel_yn8)

asking for regression-window, a culprit bug might give a hint on what's going on here, the most intriguing thing for sure is the bug seems to be intermittent, so it's more complicated that missing @orient attribute or something.

I can also reproduce the issue on Nightly71.0a1 Windows10.

Reproducible: almost 100% if Browser Toolbox was opened

Steps To Reproduce:

  1. "Enable browser chrome and add-ons debugging toolbars" and "Enable remote debugging"

  2. Enable Menu Bar or Bookmarks Toolbar with subfolders

  3. Open Browser Toolbox (Ctrl+Alt+Shift+I)

  4. Click "Bookmarks" in Menu Bar, subfolder etc...

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=94eaa02e0ea1725512893cd5139b1d200fcf4fdd&tochange=5e221240fc1d537da612973800bbe24efa50736b

Regressed by:
5e221240fc1d537da612973800bbe24efa50736b Brian Grinstead — Bug 1514926 - Convert the arrowscrollbox binding to a Custom Element r=dao

Component: Bookmarks & History → XUL Widgets
Product: Firefox → Toolkit
Blocks: 1590280
Flags: needinfo?(bgrinstead)

scrollbox of an arrowscrollbox of a menupopup has @orient=vertical which should keep its content vertically aligned (a child html:slot has display:contents so it has no effect), however inspector's computed style claims the scrollbox has -moz-box-orient: horizontal, which is missed from css rules, so I assume it goes somewhere from native XUL layout implementation (also having these lines [1] support this thesis). When I change -moz-box-orient style to vertical, then the menupoup looks normal.

My guess would be is @orient is ignored by layout for some reason, while @orient attribute is propagated from arrowscrollbox [2] to scrollbox [3].

I suppose the easiest fix will be is to make [1] unconditional (confirmed it fixes the issue, however supposedly bug 1590280 makes layout jumpy). Having said that this is something @emilio might want to look at. The weirdest thing here is opening Browser Toolbox is mystically related with the behavior: I cannot reproduce the bug having no BrowserToolbox open, and it's about 100% reproducible if it's open.

[1] https://searchfox.org/mozilla-central/source/toolkit/content/xul.css#585
[2] https://searchfox.org/mozilla-central/source/toolkit/content/widgets/menupopup.js#88
[3] https://searchfox.org/mozilla-central/source/toolkit/content/widgets/arrowscrollbox.js#18

Flags: needinfo?(emilio)

I swear we bumped into this before: https://bugzilla.mozilla.org/show_bug.cgi?id=1514926#c16, but we worked around it by setting the attr in connectedCallback instead of the constructor. I know for panel we did something as a perf optimization to avoid attribute inheritance https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/toolkit/content/widgets/panel.js#88 which might be a better workaround for this problem if there's not a viable platform fix.

(In reply to Brian Grinstead [:bgrins] from comment #10)

I swear we bumped into this before: https://bugzilla.mozilla.org/show_bug.cgi?id=1514926#c16, but we worked around it by setting the attr in > connectedCallback instead of the constructor.

which attr? I didn't catch the idea. Bug 1514926 comment 19 suggests the fix was for arrowscrollbox (the binding we deal here) is to remove delayed connected callback.

I know for panel we did something as a perf optimization to avoid attribute inheritance https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/toolkit/content/widgets/panel.js#88 which might be a better workaround for this problem if there's not a viable platform fix.

must be less destructive than to put those styles into xul.css, if nobody is against it, then I can file a patch for this workaround.

Yes, this is silly, we should figure out what's going on... If you're gonna go with the workaround, which looks like it, please file a bug with STR (which I guess will just be "revert patch, use str from one of the comments in the bug"), and I'll get to it.

Flags: needinfo?(emilio)
Depends on: 1590513

(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)

Yes, this is silly, we should figure out what's going on... If you're gonna go with the workaround, which looks like it, please file a bug with STR (which I guess will just be "revert patch, use str from one of the comments in the bug"), and I'll get to it.

done, filed bug 1590513

Flags: needinfo?(bgrinstead)
Attachment #9103329 - Attachment description: Bug 1589089 - XUL layout may ignore orient attribute on a scrollbox inside an arrowscrollbox → Bug 1589089 - Set -moz-box-orient with CSS based on the arrowscrollbox orient, since the [orient] attribute on the shadow scrollbox isn't always working

Itiel, did you happen to have the browser toolbox or similar open when it happened? Something else like that maybe?

I'm curious because now I understand what's going on when the browser toolbox is open, but I can't think of many other situations that would trigger this condition... this. Some add-ons may, I guess...

Flags: needinfo?(itiel_yn8)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)

Itiel, did you happen to have the browser toolbox or similar open when it happened? Something else like that maybe?

Yes, the toolbox is opened here most of the time lately.

Flags: needinfo?(itiel_yn8)

That's a bit of a relief. Then this is bug 1590280, just hitting a XUL layout bug on top. Let's track a solution there.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Depends on: 1592369

Going to mark this as fixed and not a dupe, since this is fixed independently of Bug 1590280.

No longer blocks: 1590280
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Assignee: nobody → surkov.alexander
Flags: qe-verify+

Conrfimed issue with 72.0a1 (2019-10-21) on Windows 10.
Fix verified with 73.0b11 on Windows 10, macOS 10.15, Ubuntu 19.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: