Closed
Bug 1366163
Opened 7 years ago
Closed 7 years ago
stylo: rendering of <video controls> and <audio controls> is wrong
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
Details
Attachments
(2 files, 2 obsolete files)
With the WIP patches in bug 1290276 comment 22, I got an incomplete rendering of <video controls>. (See the attachment.) At least "-moz-context-properties" is not supported yet (Bug 1366162). I'm not sure what else is missing.
Updated•7 years ago
|
Priority: -- → P1
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8880237 -
Attachment is obsolete: true
Attachment #8880237 -
Flags: review?(emilio+bugs)
Assignee | ||
Comment 2•7 years ago
|
||
After bug 1372876 is fixed, the remaining issue for the <video controls> is the :hover rule in [1] for the -moz-range-thumb [2]. .scrubber:hover::-moz-range-thumb, .volumeControl:hover::-moz-range-thumb { background-color: #48a0f7; } [1] http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/toolkit/themes/shared/media/videocontrols.css#234-237 [2] https://developer.mozilla.org/en-US/docs/Web/CSS/::-moz-range-thumb
Depends on: 1372876
Assignee | ||
Comment 4•7 years ago
|
||
Yes. Based on my local build, the issue in comment 2 has been fixed by bug 1375969. <video controls> looks the same on gecko and stylo now. Thanks emilio!
Status: NEW → RESOLVED
Closed: 7 years ago
Depends on: 1370797
Flags: needinfo?(tlin)
Resolution: --- → WORKSFORME
Assignee | ||
Comment 5•7 years ago
|
||
We still have some reftests failed like layout/reftests/bugs/449149-1a.html for <audio controls>. https://treeherder.mozilla.org/#/jobs?repo=try&revision=48484b37866c8674e8066a2e95826bb0653d0999&selectedJob=111173618
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Updated•7 years ago
|
Assignee: nobody → tlin
Updated•7 years ago
|
Priority: P1 → --
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 6•7 years ago
|
||
Gecko and stylo generate different frame subtree for "positionDurationBox" [1] in "data:text/html,<audio controls>". See https://pastebin.mozilla.org/9028852 stylo generated a "-moz-anonymous-flex-item" block with many newline-only text frame in it, but gecko doesn't. I haven't figured out why. [1] http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/toolkit/content/widgets/videocontrols.xml#167-169
Comment 7•7 years ago
|
||
Whitespace-only flex items are a bug, per spec, so this is a stylo issue. I suspect that we're messing up the TextIsOnlyWhitespace cached state, somehow. That path is different for stylo and non-stylo.
Assignee | ||
Comment 8•7 years ago
|
||
The difference of the frame tree described in comment 6 might be because servo doesn't blockify the children if the parent's display is "flex" where gecko has the fixup implemented in [1]. Emilio, do you have any idea how to fix that in servo? [1] http://searchfox.org/mozilla-central/source/layout/style/GeckoStyleContext.cpp#936-942,958-977
Flags: needinfo?(emilio+bugs)
Comment 9•7 years ago
|
||
I'm pretty sure servo does this in https://github.com/servo/servo/blob/master/components/style/style_adjuster.rs#L53.
Flags: needinfo?(emilio+bugs)
Comment 10•7 years ago
|
||
That being said, it does indeed seem like we're skipping it incorrectly, I can take a look at it if you want.
Comment 11•7 years ago
|
||
I think the check in GeckoElement::skip_root_and_item_based_display_fixup is just incorrect... https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dbe9482d80008a29606f68c34d70fa07ccd1a5d
Comment 12•7 years ago
|
||
Ok, that seemed to work :) Thanks a lot for finding that Ting-Yu!
Comment 13•7 years ago
|
||
I'm going to go ahead and submit that patch since I've heard Ting-Yu is on North Korea, so won't probably be available in a bit.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8894663 [details] Bug 1366163: Fix skip_root_and_item_based_display_fixup. https://reviewboard.mozilla.org/r/165816/#review171082 Oops, thanks for fixing. :-)
Attachment #8894663 -
Flags: review?(cam) → review+
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8894664 [details] Bug 1366163: Update test expectations. https://reviewboard.mozilla.org/r/165818/#review171084
Attachment #8894664 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8894663 -
Attachment is obsolete: true
Comment 20•7 years ago
|
||
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/40004a53d1d1 Update test expectations. r=heycam
Comment 21•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40004a53d1d1
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 22•7 years ago
|
||
https://hg.mozilla.org/projects/date/rev/40004a53d1d1a28b30f565ffc1329b1f28dfffd2 Bug 1366163: Update test expectations. r=heycam
You need to log in
before you can comment on or make changes to this bug.
Description
•