Closed
Bug 1382077
Opened 8 years ago
Closed 8 years ago
stylo: Restyle should not be triggered when viewport size changes while there is no media query rules
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
Attachments
(4 files)
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•8 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•8 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•8 years ago
|
||
Comment 9•8 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•8 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•8 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•8 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•8 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•8 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•8 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•8 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•8 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: 8 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
•