197263 - Unchecking "enable automatic image resizing" also disables click-to-resize

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Jesse Ruderman]

Steps to reproduce:
1. In prefs>appearance, uncheck "enable automatic image resizing".
2. Load a large image, such as
http://mozillazine.org/screenshots/screensnew/modern/browser.jpg.
3. Click the image.

Result: nothing happens.
Expected: image should resize to fit in the window.

This makes the image resizing feature all-or-nothing :(

(Also happens in Phoenix after I change the hidden pref to disable automatic
image resizing.)

Comment 1

[Jesse Ruderman]

-> varga

Comment 2

[Markus Schmaus]

At first this sounded strange to me why would you expect images to resize if you
disabled resizing. But the proposed behaviour would really be an improvement. 
If someone disabled the *automatic* image resizing he might still want to shrink
some of the images. If someone doesn't want to use it at all he simply doesn't
need to click on the images and there will be no disadvantages for him. 

Comment 3

[Mike Connor [:mconnor]]

*** Bug 211510 has been marked as a duplicate of this bug. ***

Comment 4

[Mike Connor [:mconnor]]

*** Bug 202052 has been marked as a duplicate of this bug. ***

Comment 5

[Jeremy M. Dolan]

95% of the time when I'm viewing a larger-than-my-browser-window image, I
want/need to view it 1:1. Rather than have to click the image 19 out of 20
times, I turned off auto image resize. It would be very nice to have it
available for the 1 in 20 case, however.

Comment 6

[Bernard Alleysson]

*** Bug 243650 has been marked as a duplicate of this bug. ***

Comment 7

[Adam Guthrie]

*** Bug 293795 has been marked as a duplicate of this bug. ***

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

This patch allows an embedder to disable image resizing with
browser.enable_automatic_image_resizing (like they can now), but allows users
to have images unscaled by default with resizing enabled.  I fixed SeaMonkey
only - if Firefox has pref UI for this, I'll fix it in a separate patch.

Comment 9

[Alex Bishop]

(In reply to comment #8)
> if Firefox has pref UI for this, I'll fix it in a separate patch.

It does:

http://lxr.mozilla.org/mozilla/source/browser/components/preferences/advanced.xul#121

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 192896 [details] [diff] [review]
patch

sr=jst

Comment 11

[[e-mail]]

Comment on attachment 192896 [details] [diff] [review]
patch

>+  PRBool resizeByDefault = nsContentUtils::GetBoolPref(AUTOMATIC_IMAGE_RESIZING_INITIAL_STATE);
>+
>   mImageResizingEnabled =
>     nsContentUtils::GetBoolPref(AUTOMATIC_IMAGE_RESIZING_PREF);
>-  mShouldResize = mImageResizingEnabled;
>+  mShouldResize = mImageResizingEnabled && resizeByDefault;
I would have written this as
mShouldResize = mImageResizingEnabled &&
  nsContentUtils::GetBoolPref(AUTOMATIC_IMAGE_RESIZING_INITIAL_STATE);

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 192896 [details] [diff] [review]
patch

+  PRBool resizeByDefault =
nsContentUtils::GetBoolPref(AUTOMATIC_IMAGE_RESIZING_INITIAL_STATE);

please split this line

I'm really not sure that there's a need to have both prefs...

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 192896 [details] [diff] [review]
patch

I'm not a particular fan of this pref, but ok...

Comment 14

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 192896 [details] [diff] [review]
patch

I don't like the new pref either.

Comment 15

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

Just change what the existing pref does.

Comment 16

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: diff -w of same patch

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 214089 [details] [diff] [review]
patch

r+sr=jst

Comment 18

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 214089 [details] [diff] [review]
patch

I'm not sure if this counts as an API change or not.

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 214089 [details] [diff] [review]
patch

I would say no, this is not an API change. If others disagree, please speak up.

Comment 20

[Mike Connor [:mconnor]]

*** Bug 333435 has been marked as a duplicate of this bug. ***

Comment 21

[Peter Kasting]

Fixed on branch.

/mozilla/content/html/document/src/nsImageDocument.cpp 1.138.4.5