Full-page-zoom scale factor is applied twice for SVG <image> elements

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
SVG
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Emil Ivanov, Assigned: dholbert)

Tracking

({regression})

Trunk
mozilla2.0b7
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
http://svgvspng.com/

When zooming with Firefox 3.6.11 the svg is not resized at all.

With Firefox 4.0b8pre only half of the object is resized, looks wery weird.

This is working with Chrome 7 as expected.
(Assignee)

Comment 1

7 years ago
specifically, Firefox & Chromium differ on the handling of this SVG file used at that URL:
  http://svgvspng.com/en.svg

When zoomed, Chromium increases the size of that SVG file, pushing some of it outside the browser viewport.

In Contrast, Firefox doesn't change the SVG rendering at all on full-page-zoom  -- it just leaves it occupying the full browser viewport, because the SVG has a viewBox attribute.

Firefox's behavior is correct, per the SVG spec -- we're keeping the SVG file's viewBox aligned with the browser's viewport (which is what you're supposed to do for SVG content that has a viewBox).

If you remove the viewBox attribute, we give the behavior that Emil expects -- we increase/decrease the size of the SVG as you zoom in/out.

So, this bug should be resolved INVALID, in my opinion.
The firefox 3.6.10 behaviour is correct so the png image should not change on zoom. This is a bug on trunk and my guess is that it's yet more fallout from bug 598798
Looks like we're going to need either to pass a flag to stop the zoom scaling occurring or inverse scale the GetImageTransform transform by the zoom scale factor something like this...

float cssPxPerDevPx = PresContext()->
 AppUnitsToFloatCSSPixels(PresContext()->AppUnitsPerDevPixel());

ctx->SetMatrix(GetImageTransform().Scale(cssPxPerDevPx, cssPxPerDevPx))
(Reporter)

Updated

7 years ago
Keywords: regression
(Assignee)

Comment 4

7 years ago
Ah, right -- apologies for the INVALID recommendation in comment 1.  I'd somehow missed the comparison vs. Firefox 3.6 in comment 0 -- I only noticed the comparison vs. chrome's (incorrect IMHO) behavior.

Setting as a regression from bug 598798, since I agree that's the likely cause.
Blocks: 598798
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Comment 5

7 years ago
(confirmed that this is a regression from bug 598798, with local file-revert)
Assignee: nobody → dholbert
(Assignee)

Comment 6

7 years ago
(In reply to comment #3)
> Looks like we're going to need either to pass a flag to stop the zoom scaling
> occurring or inverse scale the GetImageTransform transform by the zoom scale
> factor something like this...

The latter seems simpler.  Tested that, and it works -- patch with reftest coming up.
(Assignee)

Comment 7

7 years ago
Requesting blocking. (This is a regression w.r.t. 1.9.2)
blocking2.0: --- → ?
Summary: Page zoom not working properly with svg on this page → Full-page-zoom scale factor is applied twice for SVG <image> elements
(Assignee)

Comment 8

7 years ago
Created attachment 485374 [details] [diff] [review]
fix

This patch does what Robert suggested in comment 5 (except with a local variable to save AppUnitsPerDevPixel, since we use it multiple times in that function now).

This also removes an incorrect comment about the nonexistant variable "|destRect|" from this region of code -- that variable existed in an intermediate version of my patch for bug 598798, but not in the final version, and I forgot to remove the comment about it.  (It's regarding SVG-backed images, so even if it were accurate -- which I don't think it will be when bug 272288 lands -- it would want to come in as part of that bug's patches.)
Attachment #485374 - Flags: review?(longsonr)
Attachment #485374 - Flags: review?(longsonr) → review+
(Reporter)

Comment 9

7 years ago
(In reply to comment #4)
> Ah, right -- apologies for the INVALID recommendation in comment 1.  I'd
> somehow missed the comparison vs. Firefox 3.6 in comment 0 -- I only noticed
> the comparison vs. chrome's (incorrect IMHO) behavior.
> 
> Setting as a regression from bug 598798, since I agree that's the likely cause.

Sorry Daniel i wasnt aware that there is a possibility for svg to prevent page zoom. I also thought that Chrome is doing it right because things usually works there and Chrome did what i expected to happen with the page when zooming.

Indeed there is a comparison with Firefox trunk and branch and Chrome, trunk as i said looks very weird compared to others.
It depends on how the browser has implemented page zoom.

Firefox implements it by pretending that the display resolution is greater or smaller. So say you have a 100dpi screen then zooming will pretend that is 90 or 110 dpi depending on which way you go. If the svg was 100px wide then 100px will seem smaller or larger as it occupies fewer real dots on the screen. If an svg drawing is 100% wide/high or uses a viewBox (which does something similar but more complicated) then it just scales inversely with the zoom and the whole effect is that it looks the same.

Chrome may have implemented zooming differently as there's no specification for how browsers zoom.

Note that we're not talking SVG pan/zoom here, for which there is a specification.
blocking2.0: ? → final+
For a regular HTML document you do not want zooming to induce scrollbars by increasing the apparent size of the viewport.

I can see that for SVG documents with viewBox we might want to increase the size of the viewport referenced by viewBox by the amount of fullzoom. It's definitely not good from a UX point of view for full-zoom to appear to not work. Someone file a new bug for that?
(Assignee)

Comment 12

7 years ago
Hmm, good point.  Ok -- filed Bug 606612 on that.
(Assignee)

Comment 13

7 years ago
http://hg.mozilla.org/mozilla-central/rev/93e55d98952d
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8

Updated

7 years ago
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.