948419 - about:newtab cannot be zoomed by using Ctrl and mouse wheel

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Ada [:adalucinet]]

Reproducible on: latest Aurora (Build ID: 20131209004003)
Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0
Reproducible on: latest Nightly (Build ID: 20131209053402)
Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101 Firefox/28.0

STR:
1. Launch Firefox.
2. Navigate to at least 3 different websites.
3. Restart.
4. Open a new tab page.
5. Zoom in/out with Ctrl and mouse wheel.

Expected results: Thumbnails are zoomed.

Actual results: Thumbnails are not zoomed.

Notes:
1. Reproducible on Ubuntu and Mac OS X.
2. Not a regression, reproducible on Nightly 2013-07-24: when the Thumbnail Service landed (bug 870100).
3. Not reproducible with Ctrl and +/- buttons.

Comment 1

[Drew Willcoxon :adw]

Removing the !content->OwnerDoc()->IsXUL() term in the conditional here in nsEventStateManager::DoScrollZoom allows zooming with the mouse wheel: http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#2509

I don't remember how IsXUL() is defined, but the about:newtab page is a XUL document (chrome://browser/content/newtab/newTab.xul) with a xul:window document element with XHTML children.

Ctrl-+/- on the other hand is handled by the front-end in browser-fullZoom.js.  It doesn't check that the document is XUL, hence the difference.

Surely the mouse wheel and the +/- keys should behave the same.  Are there some XUL documents that really can't be safely full-zoomed?  Seems like the markup viewer (or whoever actually sets the full zoom) should be the one to reject incompatible documents, not the mouse wheel code.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

It would be rather odd to see Firefox UI being zoomed accidentally.

nsIDocument::IsXUL means just that the document is XULDocument.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Though, something else seems to block zoom in such case.

Comment 4

[Drew Willcoxon :adw]

(In reply to Olli Pettay [:smaug] from comment #2)
> It would be rather odd to see Firefox UI being zoomed accidentally.

Definitely, but it should be OK to zoom XUL documents shown in content, like about:newtab, depending on the specific document, I guess.  But I don't mean to suggest that I think allowing XUL documents to be zoomed should block this bug.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I think we could change that !IsXUL() check to
!document->container->IsChromeDocShell()

Comment 6

[Drew Willcoxon :adw]

Attachment: IsInChromeDocshell

Like this?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8346095 [details] [diff] [review]
IsInChromeDocshell

Yup.

Comment 8

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3610b50947e

Comment 9

[Drew Willcoxon :adw]

Meant to post this, too: https://tbpl.mozilla.org/?tree=Try&rev=2bbe4b4cf539

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c3610b50947e

Comment 11

[Gabriela [:gaby2300]]

Verified fixed using Windows 7 64 bit and Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

Comment 12

[Florin Nechita]

[testday-20140411]
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

This issue is fixed on Windows 7 SP1 x64, using the latest Beta (20140410150427).