stylo: URL values in CSSStyleSheet.insertRule and rule.style.setProperty are not recognized correctly

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
4 months ago
16 days ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 2 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 affected)

Details

Attachments

(1 attachment)

This seems to be a common failure pattern revealled by the style system tests. I'll investigate why this happens.
So they are explicitly dropped because of lack of ParserContextExtraData [1].

Looking at the code, I guess we can probably put ParserContextExtraData to Stylesheet, so that we don't need to pass in that every time when we want to parse something.

[1] https://github.com/servo/servo/blob/f37aa1292706ebcf6727aea4ef30707d8e0e0569/components/style/values/specified/url.rs#L100-L105
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #1)
> So they are explicitly dropped because of lack of ParserContextExtraData [1].

Yeah I've seen a lot of logging around that.
 
> Looking at the code, I guess we can probably put ParserContextExtraData to
> Stylesheet, so that we don't need to pass in that every time when we want to
> parse something.

Or the PerDocumentStyleData. Unless it differs between stylesheets?
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2)
> > Looking at the code, I guess we can probably put ParserContextExtraData to
> > Stylesheet, so that we don't need to pass in that every time when we want to
> > parse something.
> 
> Or the PerDocumentStyleData. Unless it differs between stylesheets?

They are definitely different between stylesheets. We would probably need one in PerDocumentStyleData as well, for style elements and attributes.
Oh, probably no need for style elements, because they have their own stylesheet.
Now I successfully make it parse, but it seems inserting rule doesn't affect the computed value... probably need to investigate what happens in the restyler.
Hmmm, it fixes much fewer issues than I thought... I have a WIP solution, but given it's not that helpful, I decide not to complete it at this moment.
Summary: stylo: URL values in rule added via CSSStyleSheet.insertRule are not recognized correctly → stylo: URL values in CSSStyleSheet.insertRule and rule.style.setProperty are not recognized correctly
Created attachment 8826171 [details] [diff] [review]
WIP patch
Blocks: 1310886
Over to xidorn with the other url stuff.
Priority: -- → P1
Blocks: 1243581
I believe this has been fixed in some dependency of bug 1343964.

The remaining failures which marked with this bug is actually bug 1356494.
Status: NEW → RESOLVED
Last Resolved: 16 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.