Closed Bug 1440258 Opened 5 years ago Closed 5 years ago

stylo: Many a11y and browser-chrome failures due to Stylo-chrome when Gecko 60 merges to Beta on 2018-03-01

Categories

(Core :: CSS Parsing and Computation, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
thunderbird_esr52 --- unaffected
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 + verified

People

(Reporter: aryx, Assigned: emilio)

References

Details

Attachments

(1 file)

Nightly-as-Beta simulation from yesterday: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b9e05c6ac8db95471d85531016bde8599a04c58&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable

There are many a11y and browser-chrome failures:

a11y log: https://treeherder.mozilla.org/logviewer.html#?job_id=163597434&repo=try
accessible/tests/mochitest/hittest/test_zoom_tree.xul | Wrong direct child of [ 'tree@id="tree" node', address: [object XULElement] ] - got '['tree@id="tree" node', address: [object XULElement], role: tree table, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleTable)]]', expected '['treecols@id="treecols" node', address: [object XULElement], role: list, address: [xpconnect wrapped nsIAccessible]]'

There are many browser-chrome failures, e.g. related to the color widget.

These failures are fallout from bug 1417138 because such a simulation push with bug 1417138 has far less failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19632bad308416ee4def096cf759ea7b6324138c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&selectedJob=163598468
Flags: needinfo?(xidorn+moz)
Gijs, do you know if there's anything beta / nightly-specific in customizableui? I don't see anything off-hand...
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #1)
> Gijs, do you know if there's anything beta / nightly-specific in
> customizableui? I don't see anything off-hand...

What there is is related to devedition, and I see the same failures there, so I'm really not sure what's happening off-hand, sorry. :-(
Flags: needinfo?(gijskruitbosch+bugs)
Priority: -- → P1
Summary: Many a11y and browser-chrome failures due to patches for bug 1417138 when Gecko 60 merges to Beta on 2018-03-01 → stylo: Many a11y and browser-chrome failures due to Stylo-chrome when Gecko 60 merges to Beta on 2018-03-01
So Aryx downloaded that build and noticed what wasn't working: When you go to "customize mode", open the "density" or "theme" dropdowns, they insta-close when you hover on them. The reason for this is that we reframe the whole window, apparently.

I still don't know why, or why it doesn't happen with Gecko, but it's easy to trigger on Gecko too with a rule like:

  :root[uidensity=compact] #customization-container { position: relative }

Which causes the whole frame tree of #customization-container to be rebuilt.

I'm doing a debug build now, though not sure I'll get to debug it proper before tomorrow morning.
So, just some notes: We're reframing the <window>, and that's causing the whole thing to get messed up as described above.

We're reframing the window because there's a XUL popupgroup like:

  XUL popupgroup@0x7f5754be0a60 state=[20000000] flags=[00000470] primaryframe=0x7f5753ff2250 refcount=3<>

with an nsPopupSetFrame that we're going to reframe too, not sure why yet.

We end up here when reframing it:

  https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/layout/base/nsCSSFrameConstructor.cpp#9834

Which ends up reframing the whole window. Why are we reframing it I'm not sure yet.
So the node I'm trying to hunt down is https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/layout/xul/nsDocElementBoxFrame.cpp#94

Now why is that getting reframed... the quest continues.
Flags: needinfo?(xidorn+moz)
See Also: → 1440506
Flags: needinfo?(xidorn+moz)
Ok, more interesting stuff, and I think we're almost there.

The reframe happens because of this bit:

4466	  if (mTextCombineUpright != aNewData.mTextCombineUpright ||
4467	      mControlCharacterVisibility != aNewData.mControlCharacterVisibility) {
4468	    return nsChangeHint_ReconstructFrame;
4469	  }


Where mControlCharacterVisibility is 0 (hidden), and it gets to 1 (visible).

Now, and that's interesting... Why would ever mControlCharacterVisibility be hidden? That's the initial value, but in XUL, the root, from which we should inherit from (maybe), it's not:

  https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/toolkit/content/minimal-xul.css#33

So I _suspect_ we're inheriting wrong initially to compute the style of that element, and when we get to restyle, because of the massive restyle that the preview gives us, we end up reframing the whole thing accidentally.

Reframing the whole thing has the unfortunate effect of going back to the beginning, which would explain why would this be perfectly reproducible.

I'm trying to confirm that theory.
Xidorn just found the missing piece of the puzzle, which is "why doesn't it fail on nightly":

01:14 <xidorn> emilio: so it sounds like it can probably be reproduced by changing the initial value of whitespace control visibility to hidden in nightly
01:15 <emilio> xidorn: it makes figuring out stuff like https://bugzilla.mozilla.org/show_bug.cgi?id=1440258#c6 a breeze instead of a pain :)
01:15 <firebot> Bug 1440258 — NEW, nobody@mozilla.org — stylo: Many a11y and browser-chrome failures due to Stylo-chrome when Gecko 60 merges to Beta on 201
01:15 <emilio> xidorn: I'm not sure, I'm still confused about why wouldn't it hit on nightly
01:16 <xidorn> emilio: https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#3151-3155
01:16 <xidorn> emilio: simply because we have different initial value in nightly
01:16 <emilio> xidorn: :o
01:17 <emilio> xidorn: that... makes total sense, horray!
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6)
> So I _suspect_ we're inheriting wrong initially to compute the style of that
> element, and when we get to restyle, because of the massive restyle that the
> preview gives us, we end up reframing the whole thing accidentally.

So after a bit of logging (child -> parent we inherit style from) this seems to be what's going on:

  <popupgroup> (0x7f842fd9ee50) -> None
  <popupgroup> (0x7f842fd9ee50) -> Some(<window id=main-window> (0x7f843035f430))
I'm suspect a lot that this bit is wrong and won't return the right value the first time:

  https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/dom/base/FragmentOrElement.cpp#184

Since the AppendNativeAnonymousChildren call won't find the element that was just created. So annoying.
Fixing this is slightly annoying, but :)
Assignee: nobody → emilio
Flags: needinfo?(xidorn+moz)
Comment on attachment 8953315 [details]
Bug 1440258: Flag doc level anonymous content instead of guessing.

https://reviewboard.mozilla.org/r/222588/#review228728

This is way better than the old setup. Thanks!
Attachment #8953315 - Flags: review?(bobbyholley) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ca30d7a852e7
Flag doc level anonymous content instead of guessing. r=bholley
https://hg.mozilla.org/mozilla-central/rev/ca30d7a852e7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.