stylo: Implement access to CSSImportRule

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 4 bugs)

53 Branch
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(11 attachments)

59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
Comment hidden (empty)
Priority: -- → P2
Assignee: nobody → xidorn+moz
(Assignee)

Comment 1

4 months ago
Note for myself:
We need to skip notifying the document about inserting of an import rule if the rule still has a pending children.
Blocks: 1243581
(Assignee)

Updated

3 months ago
Blocks: 1359217
(Assignee)

Updated

3 months ago
Depends on: 1348481
(Assignee)

Updated

3 months ago
Depends on: 1339629
(Assignee)

Updated

3 months ago
Blocks: 1357724
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=834e3de6715451a17fe7c51cb0a85b3753b63975

Comment 14

2 months ago
mozreview-review
Comment on attachment 8872218 [details]
Bug 1352968 part 0 - Fix up before adding new files.

https://reviewboard.mozilla.org/r/143684/#review147440
Attachment #8872218 - Flags: review?(cam) → review+

Comment 15

2 months ago
mozreview-review
Comment on attachment 8872219 [details]
Bug 1352968 part 1 - Add separate CSSImportRule base class.

https://reviewboard.mozilla.org/r/143686/#review147444
Attachment #8872219 - Flags: review?(cam) → review+

Comment 16

2 months ago
mozreview-review
Comment on attachment 8872220 [details]
Bug 1352968 part 2 - Move mOwnerRule to StyleSheet and use dom::CSSImportRule instead of css::ImportRule.

https://reviewboard.mozilla.org/r/143688/#review147448
Attachment #8872220 - Flags: review?(cam) → review+

Comment 17

2 months ago
mozreview-review
Comment on attachment 8872221 [details]
Bug 1352968 part 3 - Add basic CSSOM support for @import rule.

https://reviewboard.mozilla.org/r/143690/#review147452

::: layout/style/ServoImportRule.h:53
(Diff revision 1)
> +
> +  RefPtr<RawServoImportRule> mRawRule;
> +  RefPtr<ServoStyleSheet> mChildSheet;
> +};
> +
> +} // namespace mozill

mozilla
Attachment #8872221 - Flags: review?(cam) → review+

Comment 18

2 months ago
mozreview-review
Comment on attachment 8872222 [details]
Bug 1352968 part 4 - Change the direction of child stylesheet list.

https://reviewboard.mozilla.org/r/143692/#review147454
Attachment #8872222 - Flags: review?(cam) → review+

Comment 19

2 months ago
mozreview-review
Comment on attachment 8872223 [details]
Bug 1352968 part 5 - Add a param to ctor of ServoCSSRuleList for top level list.

https://reviewboard.mozilla.org/r/143694/#review147456

::: commit-message-189b4:3
(Diff revision 1)
> +Bug 1352968 part 5 - Add a param to ctor of ServoCSSRuleList for top level list. r?heycam
> +
> +@import rules can only exists in top level rule lists, and we need the

*exist

::: layout/style/ServoCSSRuleList.h:31
(Diff revision 1)
>  {
>  public:
> -  explicit ServoCSSRuleList(already_AddRefed<ServoCssRules> aRawRules);
> +  // @param aDirectOwnerStyleSheet should be set to the owner stylesheet
> +  // if this rule list is owned directly by a stylesheet, which means it
> +  // is a top level CSSRuleList. If it's owned by a group rule, nullptr.
> +  // If this param is set, the caller don't need to call SetStyleSheet.

*doesn't
Attachment #8872223 - Flags: review?(cam) → review+

Comment 20

2 months ago
mozreview-review
Comment on attachment 8872224 [details]
Bug 1352968 part 6 - Remove unused Servo_ImportRule_GetSheet.

https://reviewboard.mozilla.org/r/143696/#review147458
Attachment #8872224 - Flags: review?(cam) → review+

Comment 21

2 months ago
mozreview-review
Comment on attachment 8872225 [details]
Bug 1352968 part 7 - Make RuleHasPendingChildSheet a static method of StyleSheet.

https://reviewboard.mozilla.org/r/143698/#review147460
Attachment #8872225 - Flags: review?(cam) → review+

Comment 22

2 months ago
mozreview-review
Comment on attachment 8872226 [details]
Bug 1352968 part 8 - Construct @import rule object eagerly.

https://reviewboard.mozilla.org/r/143700/#review147464

::: layout/style/ServoCSSRuleList.cpp:270
(Diff revision 1)
> +  }
> -    mRules.InsertElementAt(aIndex, type);
> +  mRules.InsertElementAt(aIndex, type);
> +  if (type == nsIDOMCSSRule::IMPORT_RULE) {
> +    MOZ_ASSERT(!nested, "@import rule cannot be nested");
> +    ConstructImportRule(aIndex, [this](const RawServoStyleSheet* raw) {
> +      StyleSheet* sheet = mStyleSheet->GetFirstChild();

Maybe we should have a function named StyleSheet::GetMostRecentlyAddedChildSheet, that just forwards to GetFirstChild, to make it a bit clearer that that is why we're getting the first child (and we're store the child linked list in that order)?
Attachment #8872226 - Flags: review?(cam) → review+

Comment 23

2 months ago
mozreview-review
Comment on attachment 8872227 [details]
Bug 1352968 part 9 - Notify document about rule insertion only if a rule doesn't have pending child sheet.

https://reviewboard.mozilla.org/r/143702/#review147466
Attachment #8872227 - Flags: review?(cam) → review+

Comment 24

2 months ago
mozreview-review
Comment on attachment 8872228 [details]
Bug 1352968 part 10 - Update test expectation.

https://reviewboard.mozilla.org/r/143704/#review147468
Attachment #8872228 - Flags: review?(cam) → review+
(Assignee)

Comment 25

2 months ago
Servo side: servo/servo#17086
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 37

2 months ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s b22cf2f18349 -d c0a910347a2e: rebasing 398805:b22cf2f18349 "Bug 1352968 part 0 - Fix up before adding new files. r=heycam"
rebasing 398806:5205d8d42a0f "Bug 1352968 part 1 - Add separate CSSImportRule base class. r=heycam"
rebasing 398807:6e84cc987e02 "Bug 1352968 part 2 - Move mOwnerRule to StyleSheet and use dom::CSSImportRule instead of css::ImportRule. r=heycam"
merging layout/style/CSSStyleSheet.cpp
merging layout/style/CSSStyleSheet.h
merging layout/style/ServoStyleSheet.cpp
merging layout/style/ServoStyleSheet.h
merging layout/style/StyleSheet.cpp
merging layout/style/StyleSheet.h
rebasing 398808:e45a434d0e9d "Bug 1352968 part 3 - Add basic CSSOM support for @import rule. r=heycam"
merging layout/style/ServoBindingList.h
rebasing 398809:6b2b471b993f "Bug 1352968 part 4 - Change the direction of child stylesheet list. r=heycam"
merging layout/style/Loader.cpp
merging layout/style/StyleSheet.cpp
merging layout/style/StyleSheet.h
rebasing 398810:b9b78672249f "Bug 1352968 part 5 - Add a param to ctor of ServoCSSRuleList for top level list. r=heycam"
merging layout/style/ServoStyleSheet.cpp
rebasing 398811:3f7f683b64f4 "Bug 1352968 part 6 - Remove unused Servo_ImportRule_GetSheet. r=heycam"
merging layout/style/ServoBindingList.h
rebasing 398812:1895bd31f6c8 "Bug 1352968 part 7 - Make RuleHasPendingChildSheet a static method of StyleSheet. r=heycam"
merging layout/style/CSSStyleSheet.cpp
merging layout/style/StyleSheet.cpp
merging layout/style/StyleSheet.h
warning: conflicts while merging layout/style/StyleSheet.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 38

2 months ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ac37006d5ac
part 0 - Fix up before adding new files. r=heycam
https://hg.mozilla.org/integration/autoland/rev/f94836492020
part 1 - Add separate CSSImportRule base class. r=heycam
https://hg.mozilla.org/integration/autoland/rev/c1ff58ddedbb
part 2 - Move mOwnerRule to StyleSheet and use dom::CSSImportRule instead of css::ImportRule. r=heycam
https://hg.mozilla.org/integration/autoland/rev/58cf5a6058fc
part 3 - Add basic CSSOM support for @import rule. r=heycam
https://hg.mozilla.org/integration/autoland/rev/28676da16b16
part 4 - Change the direction of child stylesheet list. r=heycam
https://hg.mozilla.org/integration/autoland/rev/afe344d43816
part 5 - Add a param to ctor of ServoCSSRuleList for top level list. r=heycam
https://hg.mozilla.org/integration/autoland/rev/dd3c0e2db9cb
part 6 - Remove unused Servo_ImportRule_GetSheet. r=heycam
https://hg.mozilla.org/integration/autoland/rev/74b069ba31d6
part 7 - Make RuleHasPendingChildSheet a static method of StyleSheet. r=heycam
https://hg.mozilla.org/integration/autoland/rev/80ccd7212532
part 8 - Construct @import rule object eagerly. r=heycam
https://hg.mozilla.org/integration/autoland/rev/3a5052f5e48f
part 9 - Notify document about rule insertion only if a rule doesn't have pending child sheet. r=heycam
https://hg.mozilla.org/integration/autoland/rev/2965e8e275e6
part 10 - Update test expectation. r=heycam

Comment 39

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ac37006d5ac
https://hg.mozilla.org/mozilla-central/rev/f94836492020
https://hg.mozilla.org/mozilla-central/rev/c1ff58ddedbb
https://hg.mozilla.org/mozilla-central/rev/58cf5a6058fc
https://hg.mozilla.org/mozilla-central/rev/28676da16b16
https://hg.mozilla.org/mozilla-central/rev/afe344d43816
https://hg.mozilla.org/mozilla-central/rev/dd3c0e2db9cb
https://hg.mozilla.org/mozilla-central/rev/74b069ba31d6
https://hg.mozilla.org/mozilla-central/rev/80ccd7212532
https://hg.mozilla.org/mozilla-central/rev/3a5052f5e48f
https://hg.mozilla.org/mozilla-central/rev/2965e8e275e6
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

2 months ago
Depends on: 1368782
You need to log in before you can comment on or make changes to this bug.