(Not sure if I've found the right component for this bug.) I have whole-page zooming / image scaling disabled, so that ctrl-plus and ctrl-minus just change the text size. (I prefer to see larger text in the same layout, and I don't like the appearance of scaled images.) However, this means that when I view an image directly (not as part of an HTML page), I can't scale that image. When viewing an image directly, I'd like the usual zoom keystrokes to actually zoom the image, even though I normally use "zoom text only".
But surely the "zoom text only" option by definition only zooms text? You can resize images using the Image Zoom add-on, if that's any help: https://addons.mozilla.org/en-US/firefox/addon/image-zoom/
"Zoom Text Only" only makes sense in the context of a page which can potentially contain both text and images. When viewing an image directly, which interpretation makes more sense and provides more useful functionality: "do absolutely nothing" or "zoom the image"?
I think the suggested change makes sense for image documents. Should be an easy fix.
Hi, I'm interested in trying to fix this bug. Would you mind telling me which files or classes should I start looking in?
I believe the relevant file is http://mxr.mozilla.org/mozilla-central/source/toolkit/content/viewZoomOverlay.js#91, and perhaps http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#2517 as well.
Comment on attachment 573752 [details] [diff] [review] Patch Nice work! I think Gavin is a reasonable review for this.
Comment on attachment 573752 [details] [diff] [review] Patch I tested the patch on OSX, it works when zooming with keyboard commands (CMD++/-) but not when using scroll zooming (CTRL+scroll).
Timothy, the second link I provided in comment 5 is where mouse zooming is handled.
Created attachment 575104 [details] [diff] [review] Patch In order to find whether the document is synthetic, I added a method to nsIDocument to get mIsSyntheticDocument. I know nsDocument::GetMozSyntheticDocument exists, but it makes sense for a getter to be in nsIDocument (https://bugzilla.mozilla.org/show_bug.cgi?id=462892#c16), so I'm not too sure why that's implemented in nsDocument and not nsIDocument...
Thanks for the patch, Timothy, and sorry for the delay in responding. One thing that would help in the future is making sure that your .hgrc has the "[diff]" options from https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F , just so that the generated diffs have a bit more context. I noticed one issue with the patch as-is: it makes the View->Zoom->"Zoom Text Only" menu option not function properly on synthetic documents (you can toggle it but it doesn't have any effect on zooming behavior). I think ideally we would disable the menu option on such pages, by having FullZoom.updateMenu (defined in browser-fullZoom.js) check gBrowser.contentDocument.mozSyntheticDocument and set disabled accordingly.
Hmm, though actually, I guess it might be weird for a global toggle to be enabled/disabled based on the the current page. Maybe it's best to just leave the patch as-is.
Comment on attachment 575104 [details] [diff] [review] Patch Please also get review from a DOM peer on the content/ changes.
Comment on attachment 575104 [details] [diff] [review] Patch Review of attachment 575104 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the content/ changes.
http://hg.mozilla.org/integration/mozilla-inbound/rev/043c16767e33 Thanks for the patch, Timothy! Sorry it took so long from start to finish.