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

Status: RESOLVED FIXED

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

Description

[tgn-ff]

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

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is probably because the video controls flush layout.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1493525 - Part I, Access layout dimensions in resizevideocontrols event only r=jaws

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.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1493525 - Part II, Make videocontrols guard itself from reading the layout at the wrong time r=jaws

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

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Turned out it is not that easy because the patch is failing toolkit/content/tests/widgets/test_audiocontrols_dimensions.html

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(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 7

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/50d1b7c5b1a1
https://hg.mozilla.org/mozilla-central/rev/d8dca67e2440