1773355 - Google Docs toolbar buttons disappear when zooming the page

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: bad_screenshot.png

This bug is a regression in Nightly 83.0a1 on 2020-09-25. Here's the mozilla-central regression range:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6d45a17995e220d26df280d6af6c3bf81b69b348&tochange=b7717ee20ba9d676a6f559efbcb45027db0003c6

Unfortunately, we can't bisect to a smaller regression range because we don't save mozilla-central or autoland builds older than one year, just old Nightly builds.

Steps to reproduce

1. Load a Google Doc, such as https://docs.google.com/document/d/1fdYDDf5kSRAFKJF5Omp1xCKzlhycV1jKwaCkJPGGK7s/edit#
2. Press the Ctrl + = keyboard shortcut to zoom the page to 133% or more.

Expected result

The Google Doc toolbar buttons should get bigger and eventually overflow into a ... toolbar dropdown menu. See the attached good_screenshot.png.

Actual result

The Google Doc toolbar buttons disappear. If you mouse over the toolbar, you can still click the invisible toolbar buttons. See the attached bad_screenshot.png.

I can reproduce this bug on Windows. Akash (who found this bug) can reproduce on macOS.

Comment 1

[Chris Peterson [:cpeterson]]

Miko, do you think your fix for bug 1619370 ("YouTube video player buttons disappear in fullscreen mode when the page is zoomed") could have caused this Google Docs toolbar zooming bug? Your fix is in the regression range:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6d45a17995e220d26df280d6af6c3bf81b69b348&tochange=b7717ee20ba9d676a6f559efbcb45027db0003c6

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: good_screenshot.png

Comment 3

[Jonathan Kew [:jfkthame]]

I don't think this is related to bug 1619370; running mozregression on macOS gave me an older range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a35461d1fc07e6e855d064453363a46d32bb5a3d&tochange=bd7c35b7e234712f94f6dbb755ad983c1db8b07b

Comment 4

[Jonathan Kew [:jfkthame]]

No immediately obvious culprit in that range, but given that the GDocs buttons are rendered as fragments of a big (well, very tall and narrow) SVG image used as a sprite sheet, I wonder if clipping computations are breaking somewhere, in which case maybe:

c19d6d6768688fb627256cf2b871052f6dd7e9e2 Dzmitry Malyshau — Bug 1647918 - Refactor the clip task assignment logic in WR r=gw

might be relevant?

(There's also an SVG-related bug in the range:

5e9a7815de712ddc8ab5bb784b644a03e5f2e6b4 longsonr — Bug 1652194 - Remove SVGGenericContainerFrame r=jwatt

but AFAICS it wouldn't be applicable here at all.)

Comment 5

[Jonathan Kew [:jfkthame]]

Aha - running the "first bad revision" bd7c35b7e234712f94f6dbb755ad983c1db8b07b (2020-07-17) with gfx.webrender.force-disabled:true, the bug no longer occurs. So it does seem to be specific to WR, which makes bug 1647918 start to look quite suspicious.

Moving to Graphics:WebRender for a closer look...

Comment 6

[Timothy Nikkel (:tnikkel)]

Probably the same underlying cause as bug 1691668.

In a debug build there is a spew of:
[GFX3-]: Surface size too large (exceeds extent limit)!
Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Failed to allocate a surface due to invalid size (DTD) Size(296,37456) (t=33.6847)

The svg sprite sheet that is used is quite tall.

Comment 7

[Nicolas Silva [:nical]]

Setting image.svg.blob-image to true fixes this.

Comment 8

[Donal Meehan [:dmeehan]]

Setting 103 to Won't Fix
When do you plan to enable image.svg.blob-image? Do we need an interim fix in the meantime?

Comment 9

[Nicolas Silva [:nical]]

I haven't been involved with this work, deferring to Andrew.

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

We plan to fix the remaining issues with image.svg.blob-image in H2 and ship it to release.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Screen settings that show this bug in the default configuration.

Andrew, how far along is this? Can we mitigate this somehow? Some users can hit this in default configurations, see this for example (from @Abhinay on slack)

Comment 12

[:Gijs (he/him)]

It looks like the SVG blob image support still isn't turned on by default in Nightly. I'm unfamiliar with that project and if it will ship soon, but unless it's about to be turned on imminently and ride with 110, can we perhaps make a smaller adjustment to address the immediate issue? I'm very unfamiliar with image/ and gfx/ code, so this is probably bogus, but off-hand it looks like the error in comment 6 is from https://searchfox.org/mozilla-central/rev/4ebfb48f7e82251145afa4a822f970931dd06c68/gfx/2d/Factory.cpp#434 , and earlier https://searchfox.org/mozilla-central/rev/4ebfb48f7e82251145afa4a822f970931dd06c68/gfx/2d/Factory.cpp#337-341

// reject images with sides bigger than limit
if (extentLimit && (sz.width > extentLimit || sz.height > extentLimit)) {
  gfxDebug() << "Surface size too large (exceeds extent limit)!";
  return false;
}

extentLimit comes from the pref gfx.max-texture-size, which is set to 32767 by default. Some dumb ideas:

• can we increase the pref value? The limit is from bug 1224254 so it's 7 years old. If we doubled it, this problem goes away for a bit, AFAICT. (But I don't know if we get other problems in return.) We could potentially roll this out via normandy as well, AIUI, or try to only do so on more recent machines or ones with these hidpi displays that are more likely to hit this or w/e.
• can we apply the limit to total pixel count instead? That is, instead of limiting each side to extentLimit, can we ensure that sz.width * sz.height < extentLimit * extentLimit ? Given these svg spritesheets are only large in 1 dimension, I think this would also solve the issue here. It looks like there were issues with this with some graphics backends in 2015 (cf. bug 1224254 comment 7) but maybe webrender has helped resolve some of those?

Nical or Bas, can you comment on the feasibility of either option?

Comment 13

[jscher2000]

This issue came up on SUMO for a user with a laptop panel whose native resolution is 3840 x 2400 and whose Windows 11 system scaling needs to be 300% for legibility (any scaling over 270% causes the Google Docs button images not to appear for this user).

(In reply to Andrew Osmond [:aosmond] (he/him) from comment #10)

We plan to fix the remaining issues with image.svg.blob-image in H2 and ship it to release.

After a quick look at the open bugs, I don't think I need to give the user any warnings about this using the image.svg.blob-image preference as a workaround. I'll report back if the user reports any untoward side effects.

Comment 14

[Anna Yeddi [:ayeddi]]

This issue can also be reproduced on the window width 1280 for some levels of magnification (see below). This affects accessibility and technically fails the WCAG 2.1 Success Criteria 1.4.4 Resize text and 1.4.10 Reflow too.

Users affected are those with low vision who rely on browser zoom and screen magnification to access the content.

The inconsistent behavior range of browser zoom (cmd/ctrl++) is the following:

• ok at 100% or lower
• broken at 110%
• fixed at 120%, 133%
• broken at 150% or greater

While content authors could've avoid using hardcoded pixel positioning of their large SVG file with all the icons together and use responsive units like % or ems, the same code works well for these icons on Safari and Chrome (tested today on Mac). That's being said, none of the two other browsers listed allow for a proper toolbar overflow - it just cuts off buttons without any option to scroll it (the scrollbar at the bottom is for horizontal scrolling of the document itself). But all the icons are on their place.

Comment 15

[Jeff Muizelaar [:jrmuizel]]

FWIW, Jamie is planing on working on the dependent on deferrable-blobs issue in the next half.

Comment 16

[Jeff Muizelaar [:jrmuizel]]

I think it might be possible to increase the limit we allow here. We should be able to have Skia software surfaces that are 64k in each direction right Lee?

Comment 17

[Lee Salzman [:lsalzman]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #16)

I think it might be possible to increase the limit we allow here. We should be able to have Skia software surfaces that are 64k in each direction right Lee?

Offhand I do not believe this is possible because it won't fit into a signed 32 but integer and Skia does not allow that as such. I would need to look more to verify, but I believe that is the bad news.

Edit - SkImageInfo still constrained to receiving and reporting width and height as "int" typed, so the bad news is verified.

Comment 18

[:Gijs (he/him)]

(In reply to Lee Salzman [:lsalzman] from comment #17)

(In reply to Jeff Muizelaar [:jrmuizel] from comment #16)

I think it might be possible to increase the limit we allow here. We should be able to have Skia software surfaces that are 64k in each direction right Lee?

Offhand I do not believe this is possible because it won't fit into a signed 32 but integer and Skia does not allow that as such. I would need to look more to verify, but I believe that is the bad news.

To be clear, my suggestion in comment #12 wasn't to use the max int64 here, but just a value around 64000 (instead of around 32000). That should fit comfortably in an int32_t right, or am I missing something else?

Comment 19

[Timothy Nikkel (:tnikkel)]

I'm assuming that he means the total memory used by a 64k x 64k surface would overflow a signed 32 bit int.

Comment 20

[:Gijs (he/him)]

(In reply to Timothy Nikkel (:tnikkel) from comment #19)

I'm assuming that he means the total memory used by a 64k x 64k surface would overflow a signed 32 bit int.

That makes sense, but the surfaces in question (from gdocs) are very large in 1 dimension but tiny in the other, so we could presumably limit the width * height size and still allow either the height or the width to be larger than 32000, and that'd help resolve the issue here, right?

Comment 21

[Lee Salzman [:lsalzman]]

(In reply to :Gijs (he/him) from comment #20)

(In reply to Timothy Nikkel (:tnikkel) from comment #19)

I'm assuming that he means the total memory used by a 64k x 64k surface would overflow a signed 32 bit int.

That makes sense, but the surfaces in question (from gdocs) are very large in 1 dimension but tiny in the other, so we could presumably limit the width * height size and still allow either the height or the width to be larger than 32000, and that'd help resolve the issue here, right?

I worry that just the fallout from making the change would cause far more hard to find bugs because things make assumptions about the size (math overflows, buffer overflows, or worse) of surfaces elsewhere in the code, that would outweigh any benefits we might see. But if someone were enterprising, they might get away with such a change, but I would expect the aforementioned pain.

Comment 22

[tito]

I have experienced this issue without zooming and wanted to share some observations that may be helpful.

1. I run a two screen setup with my Apple Macbook (retina display) as one screen and a larger LG monitor as the second.
2. I only experience the missing toolbar buttons on my Macbook retina display and never on the connected monitor.
3. When the issue is in a defective state I see the missing toolbar buttons consistently on the Macbook display. Dragging the window to the connected monitor display resolves the issue. But when I drag back to the Macbook display the defect resurfaces for the same window. Dragging back to the connected monitor fixes again.
4. When the issue is in a defective state it affects all my Google doc views in Firefox.
5. The missing buttons in the toolbar is variable for the same document (see my attachments)
6. Sometimes I am able to fix the defective state by quitting Firefox and reloading. Sometimes quitting and reloading does not fix it.

Comment 23

[tito]

Attachment: Partial defective state

See my comment from Jan 26. This screenshot shows a few buttons missing.

Comment 24

[tito]

Attachment: Significant defective state

See my comment on Jan 26. This is an example showing many toolbar buttons missing.

Comment 25

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

The severity field for this bug is set to S3. However, this bug has a P1 WebCompat priority.
:gw, could you consider increasing the severity of this web compatibility bug?

For more information, please visit auto_nag documentation.

Comment 26

[Jonathan Kew [:jfkthame]]

FWIW, I tried setting an increased default for gfx.max-texture-size, and confirmed that the resulting build doesn't "lose" the Google Docs icons when zooming in (although obviously the problem could still exist in more extreme cases). Pushing the patch to tryserver doesn't show any obvious problems; the oranges there look unrelated, afaict.

As CheckSurfaceSize takes both an extents limit and an allocation size limit, we should still be protected against situations where the entire surface gets unreasonably large. So AFAICS the only issue with bumping the extent limit from 32K to 64K would be if signed 16-bit values are used somewhere to manage the dimensions (but that seems unlikely, offhand).

Maybe worth experimenting with something like this, and getting the fuzzers to try beating on it for a while?

Comment 27

[Nicolas Silva [:nical]]

A fix for the rendering issue landed in bug 1814878. There will still be some performance issue with this test case and a high risk of OOM, but the icons should render correctly on nightly, even zoomed in.

Comment 28

[James Graham [:jgraham]]

Thanks! This seems to WFM now zooming in on a high dpi display. Assuming it's also fixed for others, I suggest we file a followup bug for any remaining issues with the current solution.

Comment 29

[Anna Yeddi [:ayeddi]]

I can confirm that no icons are dissapearing, overlapping, or being cut off now for up to 400% browser zoom on macOS on Nightly 111.0a1 (2023-02-04) (64-bit) and newer which is satisfying both corresponding WCAG 2.1 Success Criteria: 1.4.4. Resize text (up to 200%) and 1.4.10. Reflow (up to 400%)

Comment 30

[Dana Lucaci, Desktop QA]

Reproduced the issue with Firefox 103.0a1 (2022-06-08) using the steps from the description on macOS 12.
The issue is verified fixed with Firefox 112.0a1 (20230228085339) and Firefox 111.0b6 (20230226190100) on Windows 10 and macOS 12.