stylo: Continue to propagate quirks mode information to Servo

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
14 days ago
2 days ago

People

(Reporter: canaltinova, Assigned: canaltinova)

Tracking

(Blocks: 3 bugs)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

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

Attachments

(2 attachments)

I've propagated quirks mode information on some places in Bug 1360488. It was a start but it isn't enough for all quirks mode. Also it didn't make all tests pass. I'll continue to work on this in this bug.
Comment hidden (mozreview-request)
(Assignee)

Comment 2

14 days ago
mozreview-review
Comment on attachment 8867542 [details]
Bug 1364746 - stylo: Continue to propagate quirks mode information to Servo

https://reviewboard.mozilla.org/r/139090/#review142318

There ere still some static `NoQuirks` in glue.rs file. But I don't think we need to pass real quirks mode information to them.

::: servo/ports/geckolib/glue.rs:1856
(Diff revision 1)
>  #[no_mangle]
> -pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, value: *const nsACString) -> bool {
> +pub extern "C" fn Servo_CSSSupports2(property: *const nsACString,
> +                                     value: *const nsACString) -> bool {
>      let id = get_property_id_from_property!(property, false);
>  
> -    parse_property(id, value, unsafe { DUMMY_URL_DATA }, structs::LengthParsingMode::Default).is_ok()
> +    // FIXME: Other parameters are dummy. I guess it's ok to pass NoQuirks here.

In here, other parameters are dummy so I passed just NoQuirks here. Do we need to get actual quirk info for that function?
(Assignee)

Comment 3

14 days ago
Finally unexpected passes! https://treeherder.mozilla.org/#/jobs?repo=try&revision=60d641348880d71ede0e4254622fff61a6e625ac

Comment 4

13 days ago
mozreview-review
Comment on attachment 8867542 [details]
Bug 1364746 - stylo: Continue to propagate quirks mode information to Servo

https://reviewboard.mozilla.org/r/139092/#review142346
Attachment #8867542 - Flags: review?(bobbyholley) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 7

12 days ago
Servo side: https://github.com/servo/servo/pull/16882

Comment 8

12 days ago
mozreview-review
Comment on attachment 8867907 [details]
Bug 1364746 - stylo: Update test expectations for unitless length quirk

https://reviewboard.mozilla.org/r/139438/#review142988
Attachment #8867907 - Flags: review?(bobbyholley) → review+

Comment 9

11 days ago
Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dad46e25b2d7
stylo: Continue to propagate quirks mode information to Servo r=bholley
https://hg.mozilla.org/integration/autoland/rev/3dda5445e08d
stylo: Update test expectations for unitless length quirk r=bholley

Comment 10

11 days ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2a534e65b84f
More reftest expectation updates. r=me
Hmm, so I was updating the test expectations without noticing the gecko side of your patch hadn't landed yet.

Nazim, if those tests I updated happen to not pass, could you please file a bug to investigate those? They're definitely related to parsing without units, so if they're not passing there's a bug servo-side.
(Assignee)

Comment 12

11 days ago
Sure, I'll look into it.

Comment 13

11 days ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0be21772f2f9
Revert "Bug 1364746: More reftest expectation updates. r=me"
Blocks: 1365629
Priority: -- → P1
I think this has been fixed.
Status: NEW → RESOLVED
Last Resolved: 2 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.