1364746 - stylo: Continue to propagate quirks mode information to Servo

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1364746 - stylo: Continue to propagate quirks mode information to Servo

I've propagated quirks mode information on some places. But we needed to pass
quirks mode to some other FFI functions to be able to handle that information
properly.

Review commit: https://reviewboard.mozilla.org/r/139092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139092/

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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?

Comment 3

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8867542 [details]
Bug 1364746 - stylo: Continue to propagate quirks mode information to Servo

https://reviewboard.mozilla.org/r/139092/#review142346

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1364746 - stylo: Update test expectations for unitless length quirk

Review commit: https://reviewboard.mozilla.org/r/139438/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139438/

Comment 6

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Comment on attachment 8867542 [details]
Bug 1364746 - stylo: Continue to propagate quirks mode information to Servo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139092/diff/1-2/

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Servo side: https://github.com/servo/servo/pull/16882

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8867907 [details]
Bug 1364746 - stylo: Update test expectations for unitless length quirk

https://reviewboard.mozilla.org/r/139438/#review142988

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a534e65b84f
More reftest expectation updates. r=me

Comment 11

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 12

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Sure, I'll look into it.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0be21772f2f9
Revert "Bug 1364746: More reftest expectation updates. r=me"

Comment 14

[Hiroyuki Ikezoe (:hiro)]

I think this has been fixed.