Closed Bug 1366144 Opened 8 years ago Closed 8 years ago

stylo: ::after pseudo element under a parent with transition didn't show up

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: shinglyu, Assigned: emilio)

References

Details

Attachments

(3 files, 2 obsolete files)

Stylo missing text screenshot
225.28 KB, image/png
Details
Gecko correct behavior screenshot
119.05 KB, image/png
Details
Bug 1366144: Test.
59 bytes, text/x-review-board-request
heycam
: review+
Details
STR: * Open http://codepen.io/juliangarnier/pen/idhuG * Observe the speed text under the "Earth" label Expected: * There will be a speed number and a line "orbit velocity" Actual: * The text are missing Notes: * The text are ::after pseudo elements .set-speed dl.infos dd span::after { content: 'Orbit Velocity'; } * The ::after pseudo elements has a parent who has transitions: .infos, .icon { transition-duration: .8s; transition-timing-function: ease-in-out; }
Blocks: stylo
No longer blocks: stylo-reftest
Yeah, I don't think this is particularly related to transitions, here you got a test case that also fails locally for me: <!DOCTYPE html> <style> div::after { color: green } div.foo::after { content: "Bar" } </style> <div></div> <script> onload = function() { document.querySelector('div').classList.add('foo'); } </script>
Assignee: nobody → emilio+bugs
cc heycam, since this may explain one of the regressions his incremental restyle optimizations have caused.
Wow, mozreview messed that up quite badly... Will rebase and see if re-pushing helps
Comment on attachment 8869454 [details] Bug 1366144: Correctly diff ::before and ::after pseudo-element styles if there's no generated content. This patch won't work as written for various reasons, but mainly that the "matches_different_pseudos" logic is executed before cascading pseudos. Since I prefer the first approach, and I don't think it's worth to tweak all that logic to hack around this, I'd prefer to go with only the first patch. Also, https://treeherder.mozilla.org/#/jobs?repo=try&revision=490592187662557e393a677c2b159907592d4aa0 is a try run with this bug + bug 1364871. I'm surprised no new generated-content-related tests pass, so I'll add the test-case in my previous comment.
Attachment #8869454 - Flags: review?(cam)
Comment on attachment 8869435 [details] Bug 1366144: Consider a display: none or empty-content ::before and ::after as not matching directly. (In reply to Emilio Cobos Álvarez [:emilio] from comment #10) > Comment on attachment 8869454 [details] > Bug 1366144: Correctly diff ::before and ::after pseudo-element styles if > there's no generated content. > > This patch won't work as written for various reasons, but mainly that the > "matches_different_pseudos" logic is executed before cascading pseudos. > > Since I prefer the first approach, and I don't think it's worth to tweak all > that logic to hack around this, I'd prefer to go with only the first patch. > > Also, > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=490592187662557e393a677c2b159907592d4aa0 is a try run > with this bug + bug 1364871. I'm surprised no new generated-content-related > tests pass, so I'll add the test-case in my previous comment. Aaand this comment should be re. this attachment, sigh, sorry about that, but mozreview made them appear in the wrong order :(.
Attachment #8869435 - Flags: review?(cam)
Attachment #8869435 - Attachment is obsolete: true
Comment on attachment 8869454 [details] Bug 1366144: Correctly diff ::before and ::after pseudo-element styles if there's no generated content. https://reviewboard.mozilla.org/r/141090/#review144820 ::: servo/components/style/matching.rs:592 (Diff revision 3) > + // an ineffective content property... > + // > + // I think it's nice to handle it here instead for symmetry with how > + // we handle display: none elements, but the other approach may be > + // ok too? > + let is_unexisting_before_or_after = Maybe "is_before_or_after_with_existing_style"?
Attachment #8869454 - Flags: review?(cam) → review+
Attachment #8869454 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c34ff92df5c3
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: