stylo: Restyle should not be triggered when viewport size changes while there is no media query rules
RESOLVED
FIXED
in Firefox 56
Status
()
People
(Reporter: xidorn, Assigned: xidorn)
Tracking
Firefox Tracking Flags
(firefox56 fixed)
Details
Attachments
(4 attachments)
layout/style/test/test_media_queries_dynamic.html checks that, the restyle generation is not updated when viewport size changes but there is no media query rules, which currently fails in Stylo. It is marked with bug 1357461 but that bug has been closed, thus file a new bug for that.
|
||
Comment 1•2 years ago
|
||
Yup, that's known[1]. I've been meant to fix it for a while already, so probably will take it in a bit, once I've gotten rid of all the other bugs. [1]: http://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/layout/base/nsPresContext.cpp#2128
|
|
||
Comment 2•2 years ago
|
||
(All the other bugs I have assigned, I mean)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
Assignee | |
Comment 8•2 years ago
|
||
try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d40acd451f53bbbedfc9274cd3b5bd76325940b9
|
||
Comment 9•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8889675 [details] Bug 1382077 part 1 - Have StyleSet::MediumFeaturesChanged return nsRestyleHint rather than a bool. https://reviewboard.mozilla.org/r/160708/#review166096
Attachment #8889675 -
Flags: review?(cam) → review+
|
|
||
Comment 10•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8889676 [details] Bug 1382077 part 2 - Move mUsesViewportUnits into nsStyleSet. https://reviewboard.mozilla.org/r/160710/#review166090 ::: layout/style/nsRuleNode.cpp:477 (Diff revision 1) > - aPresContext->SetUsesViewportUnits(true); > + // This is possible to be called on a Servo-styled document, from media > + // query evaluation outside stylesheets. Nit: "It is possible for this to be called".
Attachment #8889676 -
Flags: review?(cam) → review+
|
|
||
Comment 11•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8889677 [details] Bug 1382077 part 3 - Record viewport unit usage and generate proper restyle hint. https://reviewboard.mozilla.org/r/160712/#review166094 I notice viewport_rule.rs can call this function, and cause the bool to be set, but we don't use ViewportRule in stylo. Maybe add a comment to ViewportConstraints::maybe_new near the au_viewport_size call to say if we ever start supporting ViewportRule in Gecko, we probably want to avoid that bool being set? ::: servo/ports/geckolib/glue.rs:878 (Diff revision 1) > // > // FIXME(emilio, bug 1369984): do the computation conditionally, to do it > // less often. > let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); > > + let was_viewport_units_used = data.stylist.device().used_viewport_size(); Nit: "were_viewport_units_used" (or just "viewport_units_used") ::: servo/ports/geckolib/glue.rs:880 (Diff revision 1) > // less often. > let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); > > + let was_viewport_units_used = data.stylist.device().used_viewport_size(); > data.stylist.device_mut().reset_computed_values(); > - data.stylist.media_features_change_changed_style( > + let rule_changed = data.stylist.media_features_change_changed_style( Nit: maybe "rules_changed"?
Attachment #8889677 -
Flags: review?(cam) → review+
|
|
||
Comment 12•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8889678 [details] Bug 1382077 part 4 - Update test expectations. https://reviewboard.mozilla.org/r/160714/#review166102
Attachment #8889678 -
Flags: review?(cam) → review+
|
|
Assignee | |
Comment 13•2 years ago
|
||
Servo PR: servo/servo#17850
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 18•2 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/23c6326dd926 part 1 - Have StyleSet::MediumFeaturesChanged return nsRestyleHint rather than a bool. r=heycam https://hg.mozilla.org/integration/autoland/rev/972821835d48 part 2 - Move mUsesViewportUnits into nsStyleSet. r=heycam https://hg.mozilla.org/integration/autoland/rev/d3f7744b4ec3 part 3 - Record viewport unit usage and generate proper restyle hint. r=heycam https://hg.mozilla.org/integration/autoland/rev/333e1d30100d part 4 - Update test expectations. r=heycam
|
|
||
Comment 19•2 years ago
|
||
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d41432f808de Backed out changeset 333e1d30100d https://hg.mozilla.org/integration/autoland/rev/a95f9f156785 Backed out changeset d3f7744b4ec3 https://hg.mozilla.org/integration/autoland/rev/d173f12089ac Backed out changeset 972821835d48 https://hg.mozilla.org/integration/autoland/rev/a34cb0f468bc Backed out changeset 23c6326dd926 on request from xidorn
|
|
||
Comment 20•2 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5cf8525a1c18 part 1 - Have StyleSet::MediumFeaturesChanged return nsRestyleHint rather than a bool. r=heycam https://hg.mozilla.org/integration/autoland/rev/fe2124146276 part 2 - Move mUsesViewportUnits into nsStyleSet. r=heycam https://hg.mozilla.org/integration/autoland/rev/2d7df7d3a30f part 3 - Record viewport unit usage and generate proper restyle hint. r=heycam https://hg.mozilla.org/integration/autoland/rev/0203a69a9e6d part 4 - Update test expectations. r=heycam
|
||
Comment 21•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5cf8525a1c18 https://hg.mozilla.org/mozilla-central/rev/fe2124146276 https://hg.mozilla.org/mozilla-central/rev/2d7df7d3a30f https://hg.mozilla.org/mozilla-central/rev/0203a69a9e6d
Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•