1346481 - stylo: need to support skipping parent-display-based display fixup for anonymous boxes

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

We need this at least for the combobox display frame.  Without this, layout/reftests/forms/select/997709-2.html renders totally wrong.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This should be easily doable, we should just need to pass the relevant flag to precomputed_style_for_pseudo, either from servo directly, or from gecko if it's conditional depending on the frame.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1346481. Add stylo support for skipping parent-display-based fixup for anonymous boxes.

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

Comment 3

[Boris Zbarsky [:bzbarsky]]

This doesn't make us pass any tests, because unfortunately our rendering of <select> is still all broken.

Comment 4

[Boris Zbarsky [:bzbarsky]]

https://github.com/servo/servo/pull/15910 for the servo side.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8846244 [details]
Bug 1346481.  Add stylo support for skipping parent-display-based fixup for anonymous boxes.

https://reviewboard.mozilla.org/r/119328/#review121272

r=me, with or without the following nit that I also left in the servo PR.

::: servo/components/script_layout_interface/wrapper_traits.rs:411
(Diff revision 1)
>                              let new_style =
>                                  context.stylist.precomputed_values_for_pseudo(
>                                      &style_pseudo,
>                                      Some(data.styles().primary.values()),
>                                      &context.default_computed_values,
> +                                    false,

I think it'd be more elegant to pass CascadeFlags directly to precomputed_values_for_pseudo. We'd pass CascadeFlags::empty() here.

I don't like multiple boolean arguments in general, and should be straight-forward to do, wdyt?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8846244 [details]
Bug 1346481.  Add stylo support for skipping parent-display-based fixup for anonymous boxes.

https://reviewboard.mozilla.org/r/119328/#review121272

> I think it'd be more elegant to pass CascadeFlags directly to precomputed_values_for_pseudo. We'd pass CascadeFlags::empty() here.
> 
> I don't like multiple boolean arguments in general, and should be straight-forward to do, wdyt?

> I think it'd be more elegant to pass CascadeFlags directly to precomputed_values_for_pseudo

I agree.  I thought there was some reason it wasn't done that way.

I'll fix it.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8846244 [details]
Bug 1346481.  Add stylo support for skipping parent-display-based fixup for anonymous boxes.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fd06045940a
Add stylo support for skipping parent-display-based fixup for anonymous boxes.  r=emilio

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8fd06045940a