Closed Bug 1951011 Opened 1 year ago Closed 1 year ago

Firefox fails css/css-viewport/zoom/iframe-zoom-nested.html and iframe-zoom.sub.html

Categories

(Core :: CSS Parsing and Computation, defect)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

We fail this WPT:
https://wpt.fyi/results/css/css-viewport/zoom/iframe-zoom-nested.html
https://wpt.live/css/css-viewport/zoom/iframe-zoom-nested.html
https://wpt.live/css/css-viewport/zoom/reference/iframe-zoom-nested-ref.html

...and also this one:
https://wpt.fyi/results/css/css-viewport/zoom/iframe-zoom.sub.html
https://wpt.live/css/css-viewport/zoom/iframe-zoom.sub.html
https://wpt.live/css/css-viewport/zoom/reference/iframe-zoom-ref.html
(note, the wpt.live-hosted version of this one's testcase won't load the 3rd iframe since it's at a nonexistent domain that's only generated for local test runs, but that's not too important; you can still see the issue in the 2nd iframe.)

From a quick analysis, it looks like the test expects the iframe's content to get zoomed (in the inner document), by the zoom property that was specified on the iframe element itself (in the outer document).

We seem to scale up the iframe's viewport size using that zoom, but we don't scale up the actual content.

Summary: Firefox fails css/css-viewport/zoom/iframe-zoom-nested.html → Firefox fails css/css-viewport/zoom/iframe-zoom-nested.html and iframe-zoom.sub.html

So I imagine to implement this, we need the iframe element's effective_zoom to establish the default effective_zoom for the subdocument's root element (evenif the subdocument is cross-origin). Probably this would need to be communicated via the same channel & timing that we communicate the iframe element's computed height and width (and how we handle dynamic changes to them) to establish the size of the inner document's viewport.

Moreover, we need to do it in such a way that this default effective_zoom will "stack" on top of any explicit zoom value that's specified on the subdocument's root element, rather than being replaced by such a value. So e.g. the following data-URI example should show the text at the zoom levels described in the text (most notably at the innermost level, "Text zoomed 4x").

data:text/html,<!DOCTYPE html>
  Text at default zoom level
  <div style="zoom:2">Text zoomed 2x<br>
    <iframe srcdoc="<!DOCTYPE html><style>:root{zoom:2}body{margin:0}</style>Text zoomed 4x"></iframe>

emilio, could you check whether comment 1 sounds like a correct summary of the platform changes needed here? (And are there any gotchas/tips that you see for anyone who might pick this up?) I suspect this is relatively straightforward but I might be overlooking something.

Flags: needinfo?(emilio)

Two questions/bits-of-complexity I see here:
(1) One thing I'm hazy on: once we've somehow given the inner document the effective_zoom from the iframe (and stored it somewhere that makes sense, e.g. on the PresContext or wherever), I'm not where would be the most-sensible-spot to communicate that into the style system. Maybe you can speak to that and point at a particular spot where we might shove the passed-down effective_zoom into style, or reach out to get it from style.

Presumably somewhere in/around here, or somewhere upstream of this:
https://searchfox.org/mozilla-central/rev/81b1c51a6b1cfe1dc2a5c8b39f3346bc1d167790/servo/components/style/properties/properties.mako.rs#2775-2801

(2) It looks like we've got a special document keyword (from here) which we check for in that^ linked Rust snippet (with is_document), which we use to keep scrollbars from being zoomed here. It looks like we need to make sure that keyword keeps resolving to 1 (or more specifically, we need to be sure that scrollbars in the inner document don't get scaled by the passed-down zoom from the outer document). (We also might want to rename that keyword for clarity given that it's internal-only, since it won't be "the default zoom for our document" anymore, once we implement this special mechanism for subdocuments to take on a passed-down non-1 default zoom value.)

I think we should be able to do this more easily. In particular, something along the lines of:

  • In nsSubDocumentFrame::UpdateEmbeddedBrowsingContextDependentData, add a MaybeUpdateEmbedderZoom(), much like we have MaybeUpdateEmbedderColorScheme().
  • There look at the nsSubDocumentFrame's Style()->EffectiveZoom(), and ideally just call BrowsingContext::SetFullZoom() with PresContext()->GetFullZoom() * effectiveZoom.

I think that should "just work", and have the right semantics (we resolved that zoom on the iframe should affect the CSS viewport and so on, so basically behave the same as our full zoom).

We might need to, if someone sets the explicit full zoom some other way, to use another browsingcontext field (EmbedderFullZoom or something, and account for it in the same place we account for the current BrowsingContext::FullZoom). But I think maybe with some effort in DidSet(FullZoom) to preserve the embedder zoom we can get away without it.

Scrollbars and everything else should just work with that, as it reuses all our full zoom machinery.

Flags: needinfo?(emilio)
Depends on: 1955476

Fixed via bug 1955476.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Assignee: nobody → dholbert
You need to log in before you can comment on or make changes to this bug.