Closed Bug 1493525 Opened 2 years ago Closed 2 years ago

Some pages appear unstyled (on 1st load and on every refresh) after setting dom.ua_widget.enabled to true

Categories

(Core :: DOM: Core & HTML, defect, P2)

64 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed

People

(Reporter: tgnff242, Assigned: timdream)

References

Details

(Keywords: nightly-community, regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0
Build ID: 20180923100316

Steps to reproduce:

1. Create a new profile.
2. Open a heavy Wikipedia page. [1]
3. Refresh the page.
4. Repeat after setting dom.ua_widget.enabled to false (it's, now, true by default).

[1]: https://en.wikipedia.org/wiki/China


Actual results:

When dom.ua_widget.enabled is true, the page appears unstyled (without CSS) on the first load and on every refresh for about a second (or more on 1st load). When dom.ua_widget.enabled if false, the page appears normally.


Expected results:

The page should appear normally regardless of the value of dom.ua_widget.enabled.

I ran mozregression and it, pointed to Bug 1484048.


19:25.06 INFO: No more inbound revisions, bisection finished.
19:25.06 INFO: Last good revision: ff7afc7ea4b7ba783defb75266d0629f7a4331fc
19:25.06 INFO: First bad revision: 80d4b05cbfa93b54e82f0a7dbbd611aa2e35f24e
19:25.06 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ff7afc7ea4b7ba783defb75266d0629f7a4331fc&tochange=80d4b05cbfa93b54e82f0a7dbbd611aa2e35f24e
Blocks: 1484048
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → DOM
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(timdream)
This is probably because the video controls flush layout.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
The offenders are these calls inside adjustControlSize()

https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/toolkit/content/widgets/videocontrols.js#1700-1705

I guess it's not a problem for XBL binding because its only bound to an <audio> that is visible.

(Nice STR btw, a long article with an <audio> in the markup at the beginning.)

This is normally pretty hard to fix with the front-end code alone. Thankfully we do receive an event named "resizevideocontrols" from nsVideoFrame whenever the dimensions changes. We will just have to religiously make sure we do not call into those properties at the wrong time.
Given that the videocontrols UA Widget initializes when the DOM is inserted
(as opposed to the XBL binding only when the element is visible), the code should
not be tapping into layout until it updates.
This patch adds a few guards to the DOM elements the videocontrols holds as
properties. Any future changes that attempt to access the blacklisted layout
properties of the DOM elements will throw.

Depends on D6725
Priority: -- → P2
Turned out it is not that easy because the patch is failing toolkit/content/tests/widgets/test_audiocontrols_dimensions.html
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #5)
> Turned out it is not that easy because the patch is failing
> toolkit/content/tests/widgets/test_audiocontrols_dimensions.html

This can be fixed by pre-filling offsetHeight value with the correct intrinsic height (150px).

However, the tests have exposed some quirks in our sandbox setup.

The Proxy instance should inherit the prototype chain of the element -- this can be verified by testing |new Proxy(document.body, {}) instanceof HTMLBodyElement| -- however, this is not the case in the UA Widget scope. Attempt to use the Proxy constructor of the document global simply make the instanceof check throw.

I would need to workaround it by replacing instanceof checks with localName checks.
Comment on attachment 9011637 [details]
Bug 1493525 - Part I, Access layout dimensions in resizevideocontrols event only r=jaws

Jared Wein [:jaws] (please needinfo? me) has approved the revision.
Attachment #9011637 - Flags: review+
Comment on attachment 9011638 [details]
Bug 1493525 - Part II, Make videocontrols guard itself from reading the layout at the wrong time r=jaws

Jared Wein [:jaws] (please needinfo? me) has approved the revision.
Attachment #9011638 - Flags: review+
Attachment #9011637 - Attachment description: Bug 1493525 - Access layout dimensions in resizevideocontrols event only r=jaws → Bug 1493525 - Part I, Access layout dimensions in resizevideocontrols event only r=jaws
Attachment #9011638 - Attachment description: Bug 1493525 - Make videocontrols guard itself from reading the layout at the wrong time r=jaws → Bug 1493525 - Part II, Make videocontrols guard itself from reading the layout at the wrong time r=jaws
Pushed by tchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50d1b7c5b1a1
Part I, Access layout dimensions in resizevideocontrols event only r=jaws
https://hg.mozilla.org/integration/autoland/rev/d8dca67e2440
Part II, Make videocontrols guard itself from reading the layout at the wrong time r=jaws
https://hg.mozilla.org/mozilla-central/rev/50d1b7c5b1a1
https://hg.mozilla.org/mozilla-central/rev/d8dca67e2440
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1497514
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.