1839238 - Documentation for used_viewport_size contradicts code

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Oriol Brufau [:Oriol]]

https://searchfox.org/mozilla-central/rev/ddd233d195a5e4b1c75a972a7fd322947fa1d5e3/servo/components/style/gecko/media_queries.rs#58-63

    /// Whether any styles computed in the document relied on the viewport size
    /// by using vw/vh/vmin/vmax units.
    used_viewport_size: AtomicBool,
    /// Whether any styles computed in the document relied on the viewport size
    /// by using dvw/dvh/dvmin/dvmax units.
    used_dynamic_viewport_size: AtomicBool,

https://searchfox.org/mozilla-central/rev/ddd233d195a5e4b1c75a972a7fd322947fa1d5e3/servo/components/style/gecko/media_queries.rs#372-425

    pub fn au_viewport_size_for_viewport_unit_resolution(
        &self,
        variant: ViewportVariant,
    ) -> Size2D<Au> {
        self.used_viewport_size.store(true, Ordering::Relaxed);
        // ...
        match variant {
            // ...
            ViewportVariant::Dynamic => {
                self.used_dynamic_viewport_size
                    .store(true, Ordering::Relaxed);
                // ...
            },
        }
    }

Looks like, opposed to what the comment says, used_viewport_size is also set to true for dvw/dvh/dvmin/dvmax.

I think it's the comment what's wrong, because of https://searchfox.org/mozilla-central/rev/ddd233d195a5e4b1c75a972a7fd322947fa1d5e3/servo/ports/geckolib/glue.rs#7762-7768

    if !device.used_viewport_size() {
        return;
    }

    if dynamic_only && !device.used_dynamic_viewport_size() {
        return;
    }

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Yes, would have to dig the phab discussion but iirc it was done to not needlessly complicate invalidation.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1610815

:hiro, since you are the author of the regressor, bug 1610815, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

S4, it's just a comment fix.

Comment 4

[Daniel Holbert [:dholbert]]

(Let's track this as a followup rather than a regression, since there's no user-facing impact/bug here.)