Closed Bug 73322 (autoresize) Opened 24 years ago Closed 22 years ago

Automatic or option-based image resizing (Function to view an image resized to fit the screen)

Categories

(SeaMonkey :: UI Design, defect, P2)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
All
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: doronr, Assigned: janv)

References

Details

(Keywords: helpwanted, Whiteboard: [Hixie-PF] [imglib], [adt2])

Attachments

(10 files, 7 obsolete files)

Screenshots showing the feature in IE6 Preview
(deleted), image/png
Details
Image 1 - 1472 x 1104
(deleted), image/jpeg
Details
Image 1 - 736 x 552
(deleted), image/jpeg
Details
Image 2 - 1472 x 1104
(deleted), image/jpeg
Details
Image 2 - 736 x 522
(deleted), image/jpeg
Details
HTML to show Mozilla's scaling
(deleted), text/html
Details
Image with "T"s 320x200
(deleted), image/gif
Details
Scaling of the "T"s
(deleted), text/html
Details
fixing jst's comments
(deleted), patch
caillon
: review-
jag+mozilla
: superreview+
Details | Diff | Splinter Review
patch to fix possible binary compatibility issues
(deleted), patch
caillon
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Here is an interesting ie6 feature that sounds doable and cool: "Auto Image Resize You no longer need to scroll horizontally or vertically to view large pictures. If pictures are too large to display in the browser window, the new automatic picture resizing feature resizes the pictures automatically so they fit within the dimensions of the browser window. In addition, if you navigate to pictures that fit within the browser window and then you change the window dimensions, Internet Explorer automatically adjusts the pictures to fit the new window size. To prevent image distortion, Internet Explorer adjusts both the picture height and width, even if only a single dimension needs adjusting." While I doubt a single dimension adjusting would look good, such a feature would be neat. Perhaps in the context menu (/me ducks) for images that overflow have an "fit to browser screen size" option. Maybe even like the text size change feature, we could have certain % for that.
Keywords: helpwanted
See also bug 32646, [RFE] Full zoom for content (objects as well as text).
I'd hate trying to read the map at http://www.cuc.claremont.edu/largemap%20with%20key.htm or look at a screenshot with the image shrunk. How will users know that it's possible to zoom in on the image?
I wonder whether we can do this in JS... Basically, grab the .width and .height of the image and change them. That seems fairly doable... I'll try it tonight.
I would guess the only time you'd want to do this is if the URL being shown is an image, not an HTML page. That limits the circumstances under which this feature needs to work. Also, having some sort of pref for this is probably best, since not everyone will want things resized (though it would be convenient sometimes).
Quicktime does this to PNGs and stuff, i absolutely HATE it.
removing the "auto" part, I think if we do implement this, we should not do this automatically.
Summary: [rfe] Auto Image resize → [rfe] Image resize
To expand on timeless's comment: if you have QuickTime installed and visit a .png url with IE, the image is squished to fit on the screen. QT does ok with in-page PNGs, but I think it makes those pages take longer to load (since QT has to load).
These are Pav's bugs now.
Assignee: pnunn → pavlov
Adding dependancy on a bug in getComputedStyle(); until that's fixed we can't tell how big the margin of <body> is and so we get the sizing wrong and users still have to scroll. Alternately, we could kill the margin when we resize, but that would make the resized version look strangely different from the regular "view image" page....
Depends on: 73525
Depends on: 73847
OK, I have a patch for this. It works around the bug in getComputedStyle by using "" instead of null, and it determines the MIME type based on extension.... This whole thing currently does not work with libpr0n. For some reason libpr0n chooses not to resize the image when I change its dimensions. See bug 74313. the libpr0n issue
Depends on: 74313
Attached patch Implement this (obsolete) (deleted) — — Splinter Review
Target Milestone: --- → Future
Depends on: 75338
changing status to imglib
Whiteboard: [imglib]
If we do this, we should do it in this order: 1. Get mpt to write a UI spec. 2. Get someone to write a technical spec. I would guess that (1) would throw "context menu option" out of the window and (2) would force us to not use extensions for working out that we are showing an image. What IE does is as follows: 1. If the image is less than the size of the window, do nothing. 2. If the image is greater than the size of the window, and the mouse pointer is over the image, then show a button floating over the bottom right most visible part of the image indicating an image zoom feature. 3. If the button is clicked, make the image fit within the window. 4. If the window is resized, adjust the image. 5. If the mouse is over the image when the image is zoomed, then show the button with a reverse icon. 6. If the button is clicked, go to 1. Screenshots coming up. This is not an imglib bug IMHO. It's a navigator FE feature request.
Whiteboard: [imglib] → [Hixie-PF] [imglib]
I think this would be an *awesome* feature to include! After seeing the screenshot I'm not sure that I like IE's implementation, but I love the concept of auto image resizing. I don't necessarily think that auto would mean always having to resize the image, but I think a preferences option for using this feature would be the way to go. For instance, images displayed on their own, as JPG, GIF, or PNG and *not* as part of a webpage, that extended beyond the bounds of the current window could be resized (keeping the aspect ratio of course) so that the largest dimension fits within the viewing area. But that's just my opinion. Combine this with a context menu option so that you can override preferences for the current image and this would be sweet! .... hmmmmm....how about some icon in the status bar (maybe in that blank space beside the security button) that indicates the current image has been resized/"zoomed"? :-) Just some ideas for a really cool RFE :-)
ermh, what about http://mozilla.org/docs/dom/samples/resize/index.html ??? this is almost ready to run.
*** Bug 94721 has been marked as a duplicate of this bug. ***
*** Bug 97665 has been marked as a duplicate of this bug. ***
Let me just say how much I hate this "feature" of IE6
agreed. mpt: Could we get your opinion on this?
*** Bug 102473 has been marked as a duplicate of this bug. ***
-> me
Assignee: pavlov → jaggernaut
Blocks: 102472
Depends on: 18477
Including some comments from bug that was closed as dup: Scheduled feature work for MachV: Suggested requirements: * This feature basically automatically resizes images to fit within the current browser window. * Not on pages with mixed text & images - only when a single image is selected and displayed - should work with all common image formats - jpg, gif, png, etc. * Hovering over the image should produce a button that resizes the image to it's original size when clicked/auto resizes back on second click. Current engineering Plan: Auto scale the image, put the knowledge of the current state (not scalable, scaled, not scaled) somewhere, add code to the content context to offer a menu item which lets you switch between scaled and not scaled if an image is scalable. Scaling at the image level already works, it would need to be added to the code that currently deals with displaying a single image "page". Currently assigned to jag for planning/design, implementation estimate: 2 days. ------- Additional Comments From jag (Peter Annema) 2001-09-30 22:43 ------- > * Hovering over the image should produce a button that resizes the image > to it's original size when clicked/auto resizes back on second click. I don't think this is the best UI for exposing this feature. Until a UI has been decided upon, ignore this part of the RFE. Additionally, there should be UI that allows the user to choose whether they want the browser to auto-scale images by default.
Target Milestone: Future → ---
p2/096
Priority: -- → P2
Summary: [rfe] Image resize → Automatic Image Sizing
Target Milestone: --- → mozilla0.9.6
So here's my proposal: Have a user pref which says whether to auto-scale image documents (we create a hidden html document to show the image). For these image documents we can emit some fancy css fu (thanks Hixie!) that does exactly what we want (once bug 18477 is fixed). Alternatively we do something funky in js from the onload handler. This document would have its initial state set depending on the user pref. Under the View menu there will then be a "Fit to window" checkbox menuitem which regulates whether the image auto-scales or not. It would always be there, only enabled for image documents. The menuitem would sync itself with a user pref, which would start "on" (auto-scale), and would remember its state across sessions. The menuitem when clicked would toggle the pref and the state of the image document to update it to the new setting. Comments?
> with a user pref, which would start "on" (auto-scale) Is this so that people will discover it? I feel that auto-scaling by deafult in a vanilla install is a poor behavior... it would lead to especially odd results on images that are fairly small to start with (eg icons) that a user is trying to view.
Sorry about that, I should probably have specified "if image is taller and/or wider than the current window".
Not so much so they'll discover it as so they'll see the entire image. IMO, the odd case is wanting to see some fragment of the image enlarged, most people just want to see the picture. But then, that's a discussion for n.p.m.ui...
Additionally, since it's pref based, Netscape could have a different default than Mozilla.
r=law for jag's planning/design work
*Most* large images become harder to use/read when they are resized. And unless you include annoying pop-up-ad-like ui like IE has, users won't find out that the image was ever resized; they'll just think it was an ugly or useless image. Automatic image resizing should not be on by default.
Jesse, in my experience 99% of images that don't fit in my browser are 1280x1024 digital camera photos. At least 95% of these I just want to see the friggin photo (and in fact would print the photo on a 3in x 5in piece of photo paper). Can you actually point to examples of images for which panning with the scrollbars is a more viable method of viewing them than resizing would be? UI thought. Could we not provide a little widget on the bottom right of the image that the user could drag to resize the image? This would make the image document a bit more complicated (have to add in more markup and some js to make it work). It sounds clunky, and I don't like it all that much, but maybe someone will think of a better way of doing it....
Some types of images that often do not fit in my browser window and also resize poorly: - Large images that contain text, such as long comic strips, screenshots, maps. - Photographs of people: I want to see their faces, not their feet. - Graphics where intermediate colors are coded by alternating two colors in a chess-board pattern within a region. This is common in computer-generated images. Automatically resizing large images will also encourage web site authors to participate in two bad practices: - Authors who don't want their images to be resized (for the reasons mentioned above) will put each image in its own html page instead of linking directly to the image from the thumbnails. This will waste bandwidth, give porn site operators an excuse to make you look at more ads, and prevent the linked images bookmarklet <http://www.squarefree.com/bookmarklets/pagelinks.html#linked_images> form working on those sites. - Authors with digital cameras will post 1M images "because the user's browser will figure out how to display it", ignoring bandwidth costs, and the fact that some users have turned off automatic image resizing or use another browser. Large photographs (which usually do resize well) should only be linked to by pages containing a smaller version of the image. Finally, if I click the "show me the large version of this image" link on a web site, I want to see the large version of the image, damnit :)
Summary: Automatic Image Sizing → Automatic image resizing
-> 0.9.8
Target Milestone: mozilla0.9.6 → mozilla0.9.8
I think the `Fit to Window' menu item should be persistent across documents, because it would be a PITA having to reset it for (for example) consecutive images being dragged to the same window. For text/plain, it should wrap lines to the window width as in mail/news. And for text/html (yes, I know these are future RFEs, but I do have a point), it should shrink any image in the page which was larger than the viewport in either direction, as well as shrinking anything else which was causing horizontal scrolling. Because this last effect would tend to break page layouts for the sake of readability, and for the reasons given by Jesse (who is often right even when I'm not), I think the option should be off by default. (It will be really really cool, but it should still be off by default.)
This is not 'fit to window' for text, just stand-alone images.
Resummarizing as it seems the auto part has been unanimously decided to be a Bad Thing. How about this, for UI: A toolbar would make the resizability functions obvious, but where to put it? How about use the "Links" toolbar to do this, since images obviously won't use links. It could be renamed "Extras", or something. The current show always/when useful/never prefs for it make sense for images too... It would just have resize controls on it instead when you're on an image. IE6's UI for this is awful, so if this doesn't make 1.0, we're not at much of a loss.
Summary: Automatic image resizing → (non-) Automatic image resizing
restoring summary. Please do not re-summarize or otherwise alter bugs you don't own.
Summary: (non-) Automatic image resizing → Automatic image resizing
Marking this as blocking 112941 per trudelle's request. There are two ways to implement the back-end of this. 1) use css code suggested by Hixie, which needs bug 18477 fixed. 2) use javascript to set the width/height. If we can't get bug 18477 fixed in time I'll have to use javascript. The advantage of the css code is that, when you're resizing a window, the use of css would resize the image while you're resizing the window, whereas the javascript code will only resize the image when you're done resizing the window. I originally thought there was another problem with the javascript solution (where I'm currently sizing the image from onload, which causes scrollbar flashing, but I'll change that to set the correct attributes while emitting the image document, which should prevent the scrollbar flashing).
Blocks: 112941
fyi: bug 18477 has a patch
I agree IE's UI sucks. I also agree that I hate this feature on IE. If it wasn't auto I would like it, but IE seems to think that I always want this feature on or we don't want to resize at all. When you turn auto-resizing off, you can't resize an image! This absolutely sucks, and shows how much thought Microsoft puts into their features! My opinion: 1 pref, one UI (such as link menu, context menu, double-clicking, etc). The pref would be about autosizing and default to off. The UI would be totally different from the pref, and would allow you to do it on an individual basis. You go to an image and it doesn't fit in the screen: A) The pref would be checked and if its set to auto, it would be resized B) You can use the UI to toggle the state of the image, !!but not the pref!! We really should think hard about the UI. For now, the suggestion of putting it in the personal toolbar sounds good, but I think we need to come up with a better solution. Some options: - A mouse button and hold - Double click with a mouse button - mouseclick with a key (like spacebar) - Context menu (overused) - View menu (overused) - Links bar (is this the logical place for it?) - Floating widget like IE (annoying) - Appears when hovering (conflicts with tooltips on web pages) - etc etc etc bug 4821 talks about zoom for full content. Is the current bug just covering resizing of images viewed alone, or for any image larger than the window on a web page? If both, then the UI couldn't interfere with links and mouseovers that people might put on their page.
Double-click --- I like it! First time to "fit to screen", do it again to restore to original size. Hadn't thought of that but a wonderful idea! Perhaps a different pointer over such an image (when viewing JUST the image) - one when the image is zoomed, one when it isn't? Hmmmm...
*** Bug 116069 has been marked as a duplicate of this bug. ***
what are the chances this is gonna make 098?
-> 0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
-> future
Target Milestone: mozilla0.9.9 → Future
So being futured, is this no longer part of MachV?
Correct.
No longer blocks: 102472, 112941
Attached file Bookmarklet to implement (manual) image resizing (obsolete) (deleted) —
Ok, so since this bug was futured I started playing around. While the methods I used are somewhat crude, I created a bookmarklet as a workaround for implementing this feature (minus the "auto" part). I had some earlier versions but this is the most recent and to date it works fine on every image I've tried. It will test to verify that an image is actually being resized, and the resized image is clickable (just a hyperlink) to view the original again. There are some drawbacks (the image url is not preserved in the location bar) but otherwise it does all that I would have expected of this feature anyway. I'd appreciate any comments - although e-mail might be preferrable to keep from adding to the clutter of this bug. Thanks!
Attached file Hm, error in the last one. This one should work. (obsolete) (deleted) —
Since this feature has not been designed yet, the current spec will not reflect this. i have posted the 2nd draft to the Context Menu Revision 2 document located: http://mozilla.org/projects/ui/communicator/framework/contextmenus/cmrev2-2.html
*** Bug 137118 has been marked as a duplicate of this bug. ***
I didn't know IE had an implementation of this. I don't like the IE hovering icons. I think they are a nuisance. If I would implement this, I would do it the following way: When image is embedded in a HTML file, just make the "View Image" option available as it is now. Have a pref somewhere saying if it should be resized to fit the window or not. When viewing just the image, have "View Image to fit window" and "View image in original size" options. Double clicking could be an option when viewing just the image, but it is not an option when the image is embedded in a HTML document: what happens if the picture is displayed as part of a webpage and is a link? Just my 2 Euro Cent. Thanks to the person that duped my bug. I was looking around for over half an hour for this bug and couldn't find one.
Summary: Automatic image resizing → [RFE] Automatic image resizing (Function to view an image resized to fit the screen)
>For these image documents we can emit some fancy css fu (thanks Hixie!) that >does exactly what we want what is this CSS fu? I can't find it in this bug
Component: ImageLib → XP Apps
*** Bug 146651 has been marked as a duplicate of this bug. ***
*** Bug 163537 has been marked as a duplicate of this bug. ***
I just marked bug #163537 as a duplicate i have read through all the comments. i dont understand comment #13 it seems to rule out doing this in a context menu? Maybe people dont think there is room in the context menu, but on a web page with frames or a background image there are extra items. I would only suggest offering the additional Zoom context menu items when you have already selected view image and are viewing an image on its own with no surrounding html. I think this might help address any concerns about the context menu being cluttered. i think comment #23 agrees with the idea of only having this for lone images. as to how to easily undo this action, a check box menu item perhaps? uncheck it to return the image to the default Zoom level? It is quite annoying in Internet Explorer that you dont really have any immediate way of knowing if the image you are looking at has been autoresized I would suggest only two options Zoom ... which would pop up a dialog and allow you to choose to zoom to various percentages or to Window Height/Width (maybe even to distort the height and width, who knows what intersting hacks people might add, but i digress) and based on what the Mozilla/Netscape experts think would be the most useful Zoom option one other item such as "Fit to Window" (or perhaps "fit to width" would be better). To reiterate comment #15 it make sense to have an option to autoresize large images (but only when they are stand alone not when they are wrapped in a page) Fit Large images to window Fit All images to window Fit images to window Width Fit images to window Height A drop down menu of options with a tickbox to turn it on, off by default of course. As previously stated 'auto' bad (becuase you might not want it and because it is hard to know if the browser has gone of and just autoresized the image on you). Not sure where in preferences this could go. in response to comment #19 this feature should definately be off by default, users who dont like it wont have to use it. Another possibility is that if you are only going to allow this feature for lone images you could hijack the keyboard shortcuts for increase text size which would give a nice easy way to change the width and a status bar message could inform you of the current size and zoom level (rather than just an uninformative 'Done' as you get in IE, or 'Stopped'). (if my permission allow) i have addded the word autoresize as an alias, it is what microsoft call this feature and i think it will help people trying to find this report http://www.microsoft.com/windows/ie/using/howto/customizing/autoresize.asp I hope i am using the aliase feature correctly, i tried google and look through some of the bugzilla documentation but did not find an explanation of how it is supposed to use. It would be really cool if someone could package one of the proposed patches as an XPI, maybe that would give us a better sense of what works. I have added myself to the CC list. Feel free to mail me directly any comments or criticisms if perhaps you dont think they are relevant enough to add to bugzilla.
Alias: autoresize
I am adjusting the topic to reflect that we might go a different route than IE's intrusive Automatic resizing (bad idea IMHO) and just provide the option (good idea IMHO). ... Now that all the bugs that block this one are fixed, someone (I'm not voluntaring myself) can implement this, right? ... Alan: You have just entered the world of UI craziness and prepare to never be the same again (If you manage to keep yourself alive ;-) To summarize the following: I agree that the best place for this is the context menu, but I believe that each UI/HTML element should have its own submenu at the bottom of the context menu for element-specific options. ... No, comment #13 doesn't rule out context menus as MPT can be beaten into submission sometimes, but we have to be careful to not put too much in there. Possibly, there might be specialized context menus that use modifier keys, etc... but if we start putting more things in there, we will have to even add the kitchen sink as people will want it. Then people will complain the context menus are too confusing and we are back at square 1. So - the best solution for context menu would be a special context menu opened with a modifier key when clicking on images. There are a lot of other HTML/XUL elements that could use having extra items on the context menu, but we have to be careful. Perhaps context menus could have a submenu labeled "Extra>" or maybe labelled specifically (like "Image>" or "Frame>" for all the extra stuff relative to where you clicked on a page. ... Check boxed menu item sounds good. ... For a user like me, being able to autosize an image to the entire size of the page even if it is just a part of the web page wouldn't seem too confusing, but for the average user it would be. They wouldn't realize that doing so would unload the current web page, so I agree that leaving it off is probably the best solution. The day we have "Advanced/Intermediate/Beginner" and customized context menus, we can rethink this. ... Jag: Are you planning to do this? If not, any takers for this bug?
Summary: [RFE] Automatic image resizing (Function to view an image resized to fit the screen) → [RFE] Automatic or option-based image resizing (Function to view an image resized to fit the screen)
in the summary of this bug, you speak about "autoresize-ing" but as i see in your comments, the IE-like autoresizing is not the best thing for mozilla ... have a look at bug 163537 all of you, there i posted some issues about zooming in general; here the comment with the detailed ideas from there: ------------------------------------------------ ----------Comment 7 from Bug 163537------------- ------------------------------------------------ features for zooming: - zoom-to-fit: this is usefull to show the whole image - a part of the feature "autoresize" that is commented in bug 73322 - zoom-(1:1): this is usefull to show the real size of an image - it works perfect when the resolution of the display is setup correctly (GIMP, Corel-PhotoPaint ... have it in their preferences - don't know how mozilla handle it over imglib) - when the resolution is setup correctly, also distances of the image on the display are right /// this is not "zoom to 100%" (only when you scan something in the same resolution your display is, then it is 100%) - ask if you want a detailed explaination, my english is not perfect - zoom-in: - zoom-out: these are the two features that do not exist in IE, but are - so think i - of big importance: e.g. when you want to show a image in the double size (2:1) or in the half size (1:2), but i suggest to have a part in the preferences to specify the factor of zooming (1/2, 1/3 ...) because some people may want to zoom only a "third" in/out (4:3) and not "half" (2:1) - ask when you do not understand what i mean now how to implement this in mozilla: 1* the idea with the menu at the right-mouse-button is not bad - maybe we may have a item [Zoom] with the subitems [to fit], [to 1:1], [IN] and [OUT] - where the factor for IN and OUT can be specified in the preferences 2* or with shortkeys: when the mouse is over a image, [=]=[to fit], [1]=[to 1:1], [+]=[IN] and [-]=[OUT] 3** or/and with small buttons in the tab-title (in the right corner, just after the filename of the image) like this: _______/ [ ]image724892489248924.png [Fit][1:1][+][-] \_____ 4** in the menu [View], there should be a subitem [Image Zoom] with subitems: [to fit] [8:1] [4:1] [3:1] [2:1] [1:1] [1:2] [1:3] [1:4] [1:8] *) for standalone images and inline-images (in html ...) **) only for standalone images possible (isn't it?) ------------------------------------------------
How about this (sorry for not posting this a couple of months ago): When an image document is displayed, make the image clickable with a zoom in/out cursor (depending on the current state) and switch between scale-to-fit and 100%. Allow for some vendor pref to specify whether the image starts out in 100% or scale-to-fit. That should cover this bug, make the feature discoverable while not being too intrusive. Additional features, e.g. image scaling, can be dealt with (from a context menu) later, if at all, but shouldn't be too hard to merge with this model (anything that's not 100% will go to 100%, and from there the next click will take you to scale-to-fit).
jag: "When an image document is displayed, make the image clickable with a zoom in/out cursor (depending on the current state) and switch between scale-to-fit and 100%. Allow for some vendor pref to specify whether the image starts out in 100% or scale-to-fit." Do you mean like an hourglass cursor? Sounds good, but how could we do it without driving people crazy with changing cursors whenever they go over an image? User discovery of things like this would be best achieved with a "Welcome to Mozilla" wizard or something... like the "Welcome to Windows XP" movie Microsoft has. I don't know if I want to see a zoom cursor on all images. I guess if its kept to just viewing images as an URL as opposed to inside a web page, it would be fine. If you right click on it, it could bring up a zooming context menu, and if you left click, it could toggle between maybe 4 pre-set zoom states or something. I think all images should initially be shown at 100% though. If we want to allow a vendor pref, how do we do this without breaking the standards (maybe I misunderstood you though)? Is there anything about zooming in the CSS or DOM model? I think IE has some proprietary zooming CSS/DOM magic... Should we adopt that until there is a real standard? We should be careful not to make a standards mess again or perpetuate it. For the zooming of all images on page, etc... I think that is best taken when we make form elements, etc also zoomable - which is another bug.
Alan: By UI craziness - I wasn't implying you had no UI experience. I meant "Mozilla UI craziness". In the years Mozilla has been around, there have already been 3 developers lost in battle over the UI. The last one was stabbed 8 times by a crazed UI fanatic! (I'm just kidding). Seriously, though... UI conversations get pretty heated, and sometimes its not a pretty site, especially on IRC.
"I guess if its kept to just viewing images as an URL as opposed to inside a web page" That's what I meant by "image document" (as opposed to html document). This zooming has nothing to do with CSS or DOM, when viewing an image it's up to the user agent how to display it. What I meant by the vendor pref is that e.g. Mozilla might want "viewing images as an URL" to start at 100%, while Netscape might (note: might, not will, just using NS as an example) want them to start at "scale-to-fit" (like IE). The easiest way around this is a pref in all.js that Netscape can change in their distribution.
Keywords: nsbeta1
>make the image clickable with a zoom in/out >cursor afaict, this could be easily done _if_ javascript is enabled in browser. (then the synthetic document could simply include an onclick handler that triggers the height style) that wouldn't work if js was disabled, though... if that's acceptable, I could easily code a patch.
Hard coded unrelated functionality isn't allowed to depend on Javascript being enabled. Could do it as a bookmarklet though. Make a folder in the PT with a few functions like 'fit to screen', 'zoom out 50%', etc... could be a good workaround for people wanting this, since it's Futured.
jmd: ok, so afaict this rfe would require a lot of work if it should work with javascript disabled _and_ be triggerable on click. about the bug being futured, as I said, I can write a patch with the js dependency easily. w/o that, I can't :) unless someone else could give me a tip how this could be implemented.
While auto-resize may not be preferred by Mozilla bug/IRC/Newsgroup contributors, it is the overwhelming choice of the vast majority of browser users. People want to see the picture, not every pixel of one small corner of the picture. Once we have this capability, it should be on by default, and the handful of people who don't like it will immediately change the default and be happy thereafter. If it is off by default, most people are unlikely to know it is available, or take advantage of it.
That sounds resonable trudelle. What are the UI ideas, though? IE's is supremely annoying. It's very very important for users to realize what's happening and how to view the original image. If I'm Kodak and selling a digital camera, it's important some stupid browser doesn't mangle my high quality example JPEG and scale it to some terrible 1:.4273 ratio. Users will think the camera takes photos that look like that. If I post a large screenshot of an application I'm selling and try to show some of the functionality through the text in the screenshot, you scale that without users knowing and I'll be very pissed. Does IE have a way to, with a HTTP header I guess, turn this feature off?
Those few people who are aware of the actual pixel dimensions will know that it is being scaled, are almost certainly capable of using whatever affordance we offer to enlarge it, and are probably sophisticated enough to set the pref if they grow tired of doing so. The vast majority of users will just say "Nice picture!", and never care that they could zoom it to examine the actual pixels of Grampa's ear hair.
Peter, the point is that a picture scaled as mentioned in comment 68 will no longer look nice. It would look like crap even if we used a good scaling algorithm. Which we don't. I suppose web pages that rely on high-quality images being shown as high-quality images can just write this off as a Mozilla bug and start wrapping all those images in HTML pages to keep us from mangling them...
I agree, and because of that - I think automatic by default is out of the question. I also think that we should fix up the scaling algorithms before allowing people to resize images without setting a prefs.js option. If we scale images badly, that will be an embarrassment. There are other bugs that also depend on image scaling, and I think scaling should be improved before this bug is fixed for normal users. Is anyone able to provide the bugs(s) involved with scaling images?
Nobody is advocating changing the aspect ratio, and I'm not aware of any reason why we can't display the entire image looking as good as possible in whatever screen real estate is available, as most other programs that display images (including IE) have always done. Web sites that display large hi-res images in their own window already have to deal with the dominant browser scaling them for 95% of their users, does anyone think they are going to complain if Mozilla has the same behavior?
The question is... How does our scaling look? AFAIK, it needs work. I don't know about turning it on by default... I guess we can try it and then see the reaction. If the reaction is negative, then we can switch it the other way.
> I'm not aware of any reason Scale the following bitmap: ***** * * * to be two pixels wide. While leaving it as a recognizable letter 'T', please.
That's a pretty contrived example. Show me a large, hi-res image which has a readable letter T that is only 4 pixels high. Even lawyers don't write that small. Are you saying that Mozilla users are too fincky for even high-quality scaling, though that is apparently good enough to be the default view for Photoshop?
Attached image Image 1 - 1472 x 1104 (deleted) —
LOL! Funny example. What I'm more interested is in the general case scaling. I just thought that if we put like 50% in the title bar of the image, like they do in Photoshop: Mozilla - image.jpg [50%] Then people will know its scaled.
Attached image Image 1 - 736 x 552 (deleted) —
Here is the first image in a half size. These are tests for scaling. When the feature is implemented (or a bookmarklet, etc) we can see if the scaling works well.
Attached image Image 2 - 1472 x 1104 (deleted) —
Attached image Image 2 - 736 x 522 (deleted) —
There is a bookmarklet attached above for testing, btw (Attachment 72010 [details] by Brett Denny). You can compare the photoshop 50% scaled images I attached with the ones scaled by the bookmarklet in Mozilla.
Urgh, completely forgot about JS. I wonder if we could do something so that the (very simple) HTML document we emit gets chrome status so JS always works.
trudelle, the problem is not the height but the width. And the lowercase 'L', for example, is almost always narrow enough that scaling it down makes it disappear... You mention high-quality scaling. If that's what Mozilla did, I would have much less of an issue with this. But that's now what we do. We do crappy as-quick-as-we-can scaling.
Nobody wants crappy scaling, but that's a separate bug. Your example is more suited to a product like Acrobat, where people are expecting to read text rather than view an image. Even there though, doesn't Acrobat default to showing the full text width rather than a partial line with well-formed glyphs?
BTW, text from the image in comment #76 is unreadably fuzzy in the original, where the 't's are at least 7-8 pixels high. I recognize it as a CT plate only cuz I'm from there... Also, at full size on a 640X480 monitor, you can barery see one whole windshield wiper without scrolling.
Peter: I believe that we can start working on this bug with the current scaling and at the same time improve the scaling techniques. Anti-aliasing, bilinear scaling blah etc etc... These are techniques you can find in a lot of digital image processing books and web sites. Since both are being worked on simultaneously, we won't have to have one depend on another. When both are finished, we can make the pref to enable scaling automatically on. Advanced users can enable it in prefs.js with the knowledge that the scaling techniques need improvement. That way, users won't complain that the scaling isn't wonderful. If scaling seems good enough, then we can later switch the pref on by default without fixing scaling. Web images scale pretty nicely, but there is a big difference between a web image scaled slightly and a giant image scaled to 50% or a small image scaled the other way. See also bug 98971 about bilinear scaling. See also bug 4821 about full zooming.
Attached file HTML to show Mozilla's scaling (deleted) —
BTW: I bleeped out my license #. The text was bearly readable, but was readable imho. I felt like giving an example of Mozilla scaling with this HTML file. Although zooming in seemed fine, zooming out was a bit blocky looking. Still, its not that bad: I think that it might be good enough to enable zooming for individual images by default. The next attachment is going to be some sample text.
Attached image Image with "T"s 320x200 (deleted) —
Here is an image with text created in The Gimp.
Attached file Scaling of the "T"s (deleted) —
You will notice that on attachment 99949 [details], the scaled images are both missing a T. Obviously, this shows we don't probably want to automatically scale by default, but we can give a pref to do so. Of course, sites that didn't want their images scaled should stick images inside a wrapper web page in case someone does have the pref for autoscale enabled on either Mozilla or IE. Two prefs: 1) Turn on scaling (on by default?). 2) Automatically scale image-only to browser window (off by default).
> sites that didn't want their images scaled should stick images inside a wrapper No, no, no nono. That's disgusting content obfuscation. Please don't make developers resort to it. I request this not be enabled by default until there is a proper way for a site to disable it.
>1) Turn on scaling (on by default?). >2) Automatically scale image-only to browser window (off by default). what's the difference between these prefs?
One is to allow scaling a lone image at all. The other one is to scale it to the window automatically.
I don't see the need for two preferences... scaling should imho always be allowed - assuming that non-automatic scaling can be implemented at all.
comment 89: Why should we allow site prefs to override what our users want? The idea of serving up a lone image, and not allowing a sticky scaling pref, is to me equivalent to serving up audio content without providing volume control. And what site is going to care whether Mozilla scales or not when IE (the tool used by 90+% of their users) does? This is a case where the behavioral expectation has been set, and we clearly are not meeting it.
Wouldn't dream of overriding users wants. But there are times when scaling causes loss of content. There should be a way for a web site to indicate these times, so the default view of the image is unscaled. What about a HTTP header: X-Image: noscale X-Image: scale Mozilla/NSCP default to scaling images, though there's a pref to reserve that. The X-Image header overrides the userpref as to the DEFAULT VIEW. Just as in IE, there would be some way to toggle scaled/unscaled view while viewing the image. Most users won't know about the two different modes so it's very important a web site can indicate which mode is appropriate in the cases where it does MATTER.
Wouldn't this be simple enough to do as a user pref with three options? "Always rezise", "never resize" and "auto". First two are obvious, and "auto" would resize the image if it's too big (and leave it along if it's not as big as the visible area), *unless* the website says otherwise (by whatever means we decide upon, if we can decide :) ).
Oh, one more thing; whatever the option is on, and whatever the site says, the user should *always* be able to change between scaled and not, once the image is loaded.
Scaling down *always* discards content in the displayed image, so I'm not sure what such a header would really mean (maybe it is a hint/suggestion?). I have no problem with it, but I think it would need to be standardized or supported by IE in order to catch on. As to the prefs, I'm not sure what "always resize" would mean, it isn't obvious to me why anyone would always want to resize images. IE seems to cover all the bases with a simple checkbox "Enable Automatic Image Resizing", do we really need anything more than that?
Biesi: >assuming that non-automatic scaling can be implemented at all. I don't see why not. It would only require placing an image in the browser frame and doing CSS image resize on it (hidden from the user). The options for default image zooming can be as simple as altering the view menu as follows (removing "Text-Zoom"): -------------- View +------------+ |... | |Scaling > |+----------------+ |... ||Document Text > | +------------+|Full Document > | |Single Image > |+-----------------------+ +----------------+| 25% | | 50% | |V 75% | | 100% | | 150% | | 200% | | Custom [75%]... | | Fit to Window (Auto) | +-----------------------+ | Default... | +-----------------------+ ASCII ART! Fun! Mozilla could ship with 100% or Fit to Window (Auto) as the default depending on what we decide. The context menu would be the same. The scaling would be shown in the title bar of Mozilla. On a regular web page, the "single image" menu in the view menu would be disabled, but the context menu could still be shown for individual images without the "Default..." option. The default would be whatever the page's image intrinsic size was. "Full document" I placed in that menu is about full document zooming (after text is zoomed) and is a separate bug. I just included it here to show how that section of the view menu could be changed. Regarding comment 94 from Jeremy: That could be abused by sites. IE does it anyway, so what's the issue? Does IE allow the disabling for sites?
Comment 97: "always resize" would just mean it always resizes the image if it's too big... and ignores whatever we may use to allow thr server to indicate a preference. You could just as easily implement it using two boolean prefs: - Scale large images automatically - Allow server to override above This would give the three options I mentioned in comment 95, and would add the ability to specify what to do if the server doesn't express a preference (but still allowing the server to override it).
>The options for default image zooming can be as simple as altering the view >menu as follows (removing "Text-Zoom"): oh, ok, I didn't think of a menu command which would of course work fine. I was thinking of the click-on-the-image way, as someone suggested in this bug. which would only work if JS is enabled, afaik. however... I had an email conversation with Hixie, and it appears that the "fit to window" thing seems to be difficult to implement, at least if the "max-height:100%;max-width:100%" way is used, because the aspect ratio is not respected in some important cases. I dislike the "server override" thing, for three reasons: o) few servers will send such a header just for mozilla o) even fewer will care about whether the image is resized o) servers should not tell the user how he should view the image netdemon: I somehow dislike adding the "Scaling" submenu. I would add the items directly to the View menu - because currently, the Scaling menu would in most cases only contain one item ("Document Text"), as "Full Document" is not implemented yet (afaik). creating a submenu which most of the time only contains one item sounds bad to me.
Re: comment 100, I've whipped up a few lines of JavaScript that does the scalling stuff, including aspect ratio preservation, but obviously this is a JavaScript solution. I don't know how/what stuff you output to view images, but would it be possible to do it as a chrome:// page?
James: Server override is a separate issue/bug. biesi: > I somehow dislike adding the "Scaling" submenu ... creating a submenu which most of the time only contains one item sounds bad to me. The "Scaling" submenu would only be put in when this bug is fixed, so there would be two items in the menu ;-) > I dislike the "server override" thing, for three reasons: Its a separate issue/bug anyway. >however... I had an email conversation with Hixie, and it appears that >the "fit to window" thing seems to be difficult to implement For web pages / single images or both? ... Going with what James Ross said, I realized that people rarely would set it automatically to scale to some arbitrary number like 50%, therefore... We can allow Auto-Fit to be checked or unchecked. If Auto-Fit is checked: The image will initially be scaled to the window's max width or height (whichever would allow the image to have the right aspect ratio). "Custom..." will also be checked. The percentage isn't necessary next to "Custom..." to see in the menu since it will be displayed in the title bar of the window (except when the title bar is not present). If you clicked on "Custom...", it would show the current scale that the image was automatically set to (so you could see what it was auto-scaled to). It might also be nice to be able to unconstrain proportions with a check box within the "Custom..." dialog box and set the height and width separately (like in Photoshop) using percentage, absolute pixels, or em. If Auto-Fit isn't checked: Each image loaded will be scaled to whichever percentage selection (or "Custom...") is checked. To further explain "Custom...", when you open the dialog, it _ALWAYS_ shows the current scale regardless of whether you are in Auto-Fit mode or not. This flexibility allows people to not only auto-scale, but also to scale all images they open to a certain percentage. Although the latter is rarely necessary, it would be helpful for people browsing through a lot of identically-sized images. This brings us to my second UI suggestion (which I like better): -------------- View +------------+ |... | |Scaling > |+----------------+ |... ||Document Text > | +------------+|Full Document > | |Single Image > |+-----------------------+ +----------------+| 25% | | 50% | | 75% | | 100% | | 150% | | 200% | |V Custom... | +-----------------------+ |V Auto-Fit to Window | +-----------------------+ Prefs: We would only need one pref, which would be either set to "Auto" or to a value representing the zoom percentage. If we allowed unconstraining of proportions, then this pref would have to have a pref for the constraining checkbox and an X and Y percentage(or possibly em or px). We could probably even allow the width or height to be set auto while the other has a pixel value if proportions were unconstrained (much like CSS allows).
James: I imagine this would be done in the chrome using XUL/Javascript with none of it visible to the user that contains the image. This XUL from the user would probably be dynamically generated within XPCOM C++ and would need chrome privileges. I haven't looked into this, though, so its only a guess.
>The "Scaling" submenu would only be put in when this bug is fixed, so there >would be two items in the menu ;-) No, there would only be one most of the time. you hardly ever view "stand-alone" images. >>the "fit to window" thing seems to be difficult to implement >For web pages / single images or both? single images. I don't care about web pages. >Going with what James Ross said, I realized that people rarely would set it >automatically to scale to some arbitrary number like 50%, therefore... We can >allow Auto-Fit to be checked or unchecked. I see no need for anything other than Auto-Fit.
>I don't know how/what stuff you output to view images, but >would it be possible to do it as a chrome:// page? No. The document for single images is generated in C++ Code, so all you could do is maybe give the script chrome privileges in C++ code (not like I know how that can be done); however, I am not sure if mstoltz would agree to such a solution.
Re comment 103, comment 105: Ok, how I see it is that if we can't/don't get chrome status for the image viewing document (the one generated for single images), or somehow always jave JavaScript enabled, we don't stand a chance of doing the image auto-fit stuff. Unless we want to make the auto-fit dependant on the user having JavaScript enabled, which I don't think would be a good idea, IMO.
>>The "Scaling" submenu would only be put in when this bug is fixed, so there >>would be two items in the menu ;-) > >No, there would only be one most of the time. you hardly ever view "stand- >alone" >images. I already said that it could be disabled when no image is available and not removed from the menu. >>>the "fit to window" thing seems to be difficult to implement >>For web pages / single images or both? > >single images. I don't care about web pages. If its single images, then I don't see how it would be very hard. It might be time-consuming, but it won't be hard. >>Going with what James Ross said, I realized that people rarely would set it >>automatically to scale to some arbitrary number like 50%, therefore... We >>can allow Auto-Fit to be checked or unchecked. > >I see no need for anything other than Auto-Fit. I would like something else besides that. I don't think we need to go into anything like separate X and Y scaling as that's really something for an image editor, but in terms of just raw percentages that maintain the image proportions, I would like to see that. It also solves the problem of how to change an auto-scaled image back to the original size and would make it a bit more functional than IE's implementation. All we need is one pref which would contain either a percentage - such as "75%", "100%" (default), "22%" (using custom scaling), or auto. BTW: What is going to happen when the window is re-sized, is the image going to be re-sized accordingly? This gives another argument for more options. You can resize the window to the scaling you like, then remove auto so that you can resize the window while leaving the image at the correct scaling. Perhaps - to flip back and forth between a page and an image.
>No, there would only be one most of the time. you hardly ever view "stand- >alone" images. [Speak for yourself ;-)] If you don't like it being grayed, then we could put the "Single Image" submenu directly in the view menu for now until that Full Zooming bug is fixed.
>>No, there would only be one most of the time. you hardly ever view "stand- >>alone" images. >I already said that it could be disabled when no image is available and not >removed from the menu. does not address the issue at all. the item is usually greyed out and therefore just as useless. >If its single images, then I don't see how it would be very hard. It might be >time-consuming, but it won't be hard. I was referring to the CSS-driven approach, rather than using JS. >It also solves the problem of how to >change an auto-scaled image back to the original size um, how so? just use whatever you used to scale it. btw, I now know a way to give that page chrome privileges, I just have to test it. >All we need is one pref which would contain either a percentage - such >as "75%", "100%" (default), "22%" (using custom scaling), or auto. (I'd use an int pref and use 0 for auto if we really want to take that route) >BTW: What is going to happen when the window is re-sized, is the image going >to be re-sized accordingly? definitely. at least if set to auto. >If you don't like it being grayed, then we could put the "Single Image" >submenu directly in the view menu for now until that Full Zooming bug is fixed. thereby eliminating the Scaling submenu, right?
A technical point. There's no reason we can't do the resizing in JS. The image document does not have chrome priveleges. So? We can put the JS in chrome and only trigger it for image documents. Basically a bubbling onclick handler on the <browser> that only triggers if we're looking at an image document.
biesi: >>It also solves the problem of how to >>change an auto-scaled image back to the original size > >um, how so? just use whatever you used to scale it. > >btw, I now know a way to give that page chrome privileges, I just have to >test it. I mean if its autoscaled, we need a way to make it return to 100% like IE. Multiple percentage values is the logical next step. >>All we need is one pref which would contain either a percentage - such >>as "75%", "100%" (default), "22%" (using custom scaling), or auto. > >(I'd use an int pref and use 0 for auto if we really want to take that route) Agreed, although a float might be better since people might want to scale using fractions of percents. >>If you don't like it being grayed, then we could put the "Single Image" >>submenu directly in the view menu for now until that Full Zooming bug is >fixed. > >thereby eliminating the Scaling submenu, right? Yes, until the full zooming bug is fixed (bug 4821 btw). ... I agree this should be done in chrome JS. Will we still need a slight XPCOM hack, though, to actually display the chrome XUL document containing the JS when an image is viewed?
biesi: And -1 could turn off zooming images altogether. This probably wouldn't be of any use to end-user but might come in handy if a developer ever needs to turn the feature off for whatever reason.
> We can put the JS in chrome and >only trigger it for image documents. Basically a bubbling onclick handler on >the <browser> that only triggers if we're looking at an image document. that is correct... it sounds like a hack to me though... > Will we still need a slight XPCOM >hack, though, to actually display the chrome XUL document containing the JS >when an image is viewed? !????? not at all
All these additions are way too much overkill; I still don't see any common need for anything more than auto shrink to fit the window, either on or off. I still don't understand how 'always resize' is any different from auto/on.
Peter: >I still don't understand how 'always resize' is any different from auto/on. What are you referring to? I don't understand the statement.
Let me summarize the current state of this bug, as I see it: o) Just using CSS to auto-size the image to the window size will not work, because the aspect ratio will be wrong in some cases. o) So, JS must be used. this will (I think) best be done from nsBrowserStatusHandler.js and get invoked when an image (detected by mimetype image/* probably) gets loaded. it finds the image in the document (which either needs to get an id or getElementsByTagName must be used) and sizes it to the window size. (window.width or something). it checks a preference before it does that. this would also allow zooming only to a percentage rather than window, if that's wanted this will not automatically resize the image if the window is made smaller, though. (addEventListener can maybe be used to auto-adjust, if that's wanted) ok, so much for the automatic stuff. manually changing orig.size/auto-size is more difficult, but bz seems to know how to do this :) question is though, is this wanted. fwiw, I agree with trudelle that only fit-to-window is needed, and that automatically sizing is enough, with a setting in the pref dialog somewhere.
Note that this would require us to override the JS pref, just like it requires us to override the image pref.
Brian: see comment #97 and comment #99. There is no need here for any more than the simple yet powerful behavior that is in IE, which is to dynamically display the largest possible view of the image within the current window size, while maintaining the correct aspect ratio. It should be controlled by a simple binary pref, on by default because it is what most users want yet would probably not discover on their own. Those who really want to see pixels rather than pictures are likely advanced enough to disable the pref.
When I use IE I often toggle this feature on the fly several times per picture, so I don't think a prefs panel pref is appropriate. Could we show a button below the picture to toggle it? That would actually probably be easier to implement, too.
> That would actually probably be easier to implement, too. It would actually be virtually the same amount of work. both things require js to be definitely enabled, and after that it's just a matter of which JS Code/HMTL Code to emit. (the button/link to toggle it could be slightly more difficult, because it has to be taken care of localization (or artwork, depending on the implementation), and because not only JS, but also HTML code needs to be emited) on the other hand, the button way could save a reading of preferences, but that's like 4 lines of simple code. (and prefs should probably be read anyway). and if the way bz mentioned is used, the prefs way is far easier.
The way I used would work just fine with the button.... Just check that it's an image document _and_ the click target is the button (we'd give the button an ID). Generating a standard <input type="button"> as part of the image document is trivial (yes, it requires a few lines of c++, but if we decide that's what we want to do it's trivial to do it).
> Generating a standard <input type="button"> as part of the image document >is trivial sure. it's the other part that I don't know about :) sure, feel free to do it that way. I just have no idea how that can be accomplished :) (not to mention that it's an xul/js patch mostly, I think. which I no longer create.)
Summary: [RFE] Automatic or option-based image resizing (Function to view an image resized to fit the screen) → Automatic or option-based image resizing (Function to view an image resized to fit the screen)
A button would be really a poor UI choice. If you are going to use it to test the code, feel free... But please replace it with something else for the final patch. Even a tooltip or status message that said, "Press CTRL + and CTRL -" and linked those key events up to image resizing would be better in my opinion. Of course, that is only just an example of something better, and I still think it sucks.
Actually, I think that's a fantastic idea. Parity with text zoom, and a way to notify users without fudging with the content view area. There's one issue though. How do toggle auto-zoom. (ie, fit to window)
cc patricec for UE input.
The functionality described in #0 sounds good. After speaking with Jag, it seems that a magnifying glass to plus/minus the image is clear and simple.
nsbeta1+/adt2 per the nav triage team.
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [Hixie-PF] [imglib] → [Hixie-PF] [imglib], [adt2]
I think I can do that in XBL. Jag, want to give me the bug ?
Actually, Jan Varga has this bug, talk to him.
Assignee: jaggernaut → varga
Attached patch patch to implement this feature (obsolete) (deleted) — — Splinter Review
bugs: 1. custom cursors are not yet supported by layout (bug 38447) 2. the resizing when window resizes is not instant, resize event is delayed by a timer in layout (bug 114649) 3. type ahead find should not trigger in image documents 4. the view menu item is not yet implemented
Attachment #29406 - Attachment is obsolete: true
Attachment #72010 - Attachment is obsolete: true
Attachment #72061 - Attachment is obsolete: true
Attached patch new patch (obsolete) (deleted) — — Splinter Review
I broke progressive rendering with previous patch This one should handle it.
Attachment #111402 - Attachment is obsolete: true
Attached patch cleanup (obsolete) (deleted) — — Splinter Review
jag could you r/sr ?
Attachment #111404 - Attachment is obsolete: true
Attachment #111423 - Flags: superreview?(jaggernaut)
Attachment #111423 - Flags: review?(caillon)
Attachment #111423 - Flags: review?(caillon)
curious, what will this mean for embeddors? will they see the image resizing UI?
instead of using the static cid for the string bundle service, can you use the contract id? NS_STRINGBUNDLE_CONTRACTID
Attachment #111423 - Flags: review?(jst)
I changed it, thanks for pointing it out
So if we can get the cursor icon working, embedding would get that part of the (visible) UI; clicking and keybindings (can these be configured/localized?) should work for them, and hooking up a menu item isn't that much work. If we can't get the cursor working in time, what would you say to using tooltips to describe what'll happen when you click (and give the cursor some "clickable content" shape)?
Comment on attachment 111423 [details] [diff] [review] cleanup - In nsIImageDocument.idl: + /* Resize the image to fit visible area. */ + void resizeImage(); How about naming this "shrinkToFit", or something like that? + /* A helper method for switching between states. + * The switching logic is as follows. If the image has been resized + * restore image original size, otherwise if the image is overflowing + * current visible area resize the image to fit the area. + */ + void resizeOrRestoreImage(); I'm not convinced this belongs in this API, you're exposing everything the caller needs in order to make this decision w/o this helper, I don't really see the need for this method. If you do end up keeping this method, then I'd suggest renaming it to "toggleImageSize()" in stead. As you can tell, I don't really like the word "resize" here, it doesn't tell you what'll happen in all cases, IMO. - In the declaration of nsImageDocument: + nsWeakPtr mContainer; Couldn't this use nsDocument::mDocumentContainer in stead? Looks like they're both pointing to the docshell that the document is in, and mDocumentContainer is already initialized by the base class... - In nsImageDocument::CheckOverflowing(): + nsCOMPtr<nsIPresShell> shell; + GetShellAt(0, getter_AddRefs(shell)); + if (shell) { ... + } return NS_OK; } How about returning NS_OK early if !shell, to avoid indenting the bulk of the method body? - In nsImageDocument::StartLayout(): - if (scrollableContainer) { + if (scrollableContainer) scrollableContainer->ResetScrollbarPreferences(); - } I don't like seeing changes like this (there are others, just like this in this diff too), while braces around one-line if statements have no meaning to the compiler, they do make the code more robust in that they make it much harder to incorrectly add code and think it's part of the if block, and so on. We've had smoketest blocker bugs due to missing braces around one-line conditional statements, so please don't remove them, and if you have the energy, please do add them in the new code you've written in this file. They *do* cause problems now n' then, so please don't open that door any wider. +NS_EXPORT nsresult +NS_NewImageDocument(nsIDocument** aResult) I know you probably copied this from somewhere, but there's no reason at all to export this function, just make it return nsresult and nothing else... - In nsIDOMClassInfo.h: // HTML classes eDOMClassInfo_HTMLDocument_id, + eDOMClassInfo_ImageDocument_id, eDOMClassInfo_HTMLCollection_id, Please add eDOMClassInfo_ImageDocument_id to the *end* of this list of id's. r/sr=jst if you fix that.
Attachment #111423 - Flags: review?(jst) → review+
Attached patch fixing jst's comments (deleted) — — Splinter Review
>+ void resizeImage(); >How about naming this "shrinkToFit", or something like that? done >+ void resizeOrRestoreImage(); >I'm not convinced this belongs in this API, you're exposing everything the >caller needs in order to make this decision w/o this helper, I don't really >see the need for this method. If you do end up keeping this method, then I'd >suggest renaming it to "toggleImageSize()" in stead. As you can tell, I don't >really like the word "resize" here, it doesn't tell you what'll happen in all >cases, IMO Currently we need this at two places, in nsImageDocument.cpp and nsContextMenu.js. We will need it possibly in other XUL, like additional view menu item. So I thought it's good to have a method for this logic I explained this to jst on IRC and he thinks it's ok now. >+ nsWeakPtr mContainer; >Couldn't this use nsDocument::mDocumentContainer in stead? Looks like they're >both pointing to the docshell that the document is in, and mDocumentContainer >is already initialized by the base class... yeah, it also worked with mDocumentContainer so I removed mContainer >+ nsCOMPtr<nsIPresShell> shell; >+ GetShellAt(0, getter_AddRefs(shell)); >+ if (shell) { >... >+ } >How about returning NS_OK early if !shell, to avoid indenting the bulk of the >method body? done >- if (scrollableContainer) { >+ if (scrollableContainer) > scrollableContainer->ResetScrollbarPreferences(); >- } > >...so please don't remove them, and if you have the energy, please do >add them in the new code you've written in this file. done >+NS_EXPORT nsresult >+NS_NewImageDocument(nsIDocument** aResult) >just make it return nsresult and nothing else... done > // HTML classes > eDOMClassInfo_HTMLDocument_id, >+ eDOMClassInfo_ImageDocument_id, > eDOMClassInfo_HTMLCollection_id, >Please add eDOMClassInfo_ImageDocument_id to the *end* of this list of id's. done
Attachment #111423 - Attachment is obsolete: true
Comment on attachment 111497 [details] [diff] [review] fixing jst's comments moving flags
Attachment #111497 - Flags: superreview?(jaggernaut)
Attachment #111497 - Flags: review+
Attachment #111423 - Flags: superreview?(jaggernaut)
Comment on attachment 111497 [details] [diff] [review] fixing jst's comments + if (charCode == 0x2B) { + else if (charCode == 0x2D) { Will these work on all platforms, in all locales, with all keyboards?
hmm, I'm not sure if it works on all platforms. it would be probably a good idea to make it localizable, I agree
Status: NEW → ASSIGNED
Comment on attachment 111497 [details] [diff] [review] fixing jst's comments sr=jag
Attachment #111497 - Flags: superreview?(jaggernaut) → superreview+
landed
Given that 0x2B == '+' and 0x2D == '-', I would think that it works everywhere... however, I'm no expert with that stuff, so maybe it doesn't.
Jan, when jst meant move things to the end of the lists, he meant the end of the _list_, not the end of the _section_. See: http://lxr.mozilla.org/mozilla/source/dom/public/nsIDOMClassInfo.h#243 and http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#778 Please fix this in both places, ASAP. Thanks.
Comment on attachment 111497 [details] [diff] [review] fixing jst's comments Minusing, for the time being because unlike what this patch claims, it does NOT fix (all of) jst's comments.
Attachment #111497 - Flags: review+ → review-
I can't believe that this is so *important*.
>See: >http://lxr.mozilla.org/mozilla/source/dom/public/nsIDOMClassInfo.h#243 >and >http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#778 I'm not sure if this makes sense at all (putting after SVG classes), since nsImageDocument inherits from nsHTMLDocument.
> I can't believe that this is so *important*. I can't believe that we shipped 1.0 either. But since then, binary compat has been important. Life's hard like that. ; ) > since nsImageDocument inherits from nsHTMLDocument. That's irrelevant as far as the classinfo id ordering is concerned. The point is, the order of this list is not dictated by "sense" as in logical connections but by one simple requirement -- adding a new item should be binary-compatible with the previous list, which means the new item must be at the end. Please make sure to fix this before we're anywhere close to shipping a milestone with this change.
Flags: blocking1.3b?
I'm really sorry for being ignorant, but it was not so clear from comment 146. I was not aware of this issue.
Attachment #111921 - Flags: superreview+
Comment on attachment 111921 [details] [diff] [review] patch to fix possible binary compatibility issues r=caillon.
Attachment #111921 - Flags: review+
> I can't believe that this is so *important*. I'd like to think that reviewer comments, especially when made by module owners, are always important. And I thought that my references to the comments that explicitly explained this would have made it pretty obvious as to why the change was all the more important. Thanks for fixing this.
ok, I was just confused by your reference to: http://lxr.mozilla.org/mozilla/source/dom/public/nsIDOMClassInfo.h#243 because that's not the end of list.
Microsoft has a meta tag for IE's Image Toolbar (and "Smart"Tags). Should Mozilla follow the meta tag(s)?
"Microsoft has a meta tag for IE's Image Toolbar (and "Smart"Tags). Should Mozilla follow the meta tag(s)?" Yes, please do support this.
Mozilla has no image toolbar, and images don't have meta tags.
Is this bug fixed now?
Almost fixed, except magnifying glass cursor and the view menu item.
What about "Fit to window" for mail windows ... I get lots of email with pictures that are MUCH larger thanthe VIEWABLE area on the 3-pane view ... can we turn it on in the mail window ??? (So far I have not seen it work there) WinXP Build 2003012008
Why is this pref-based, by the way? We have too many prefs as it is, we shouldn't be adding more. In fact the entire "Appearance" pref panel should be removed (along with all the prefs on that panel), not bloated further.
Severity: enhancement → trivial
> Why is this pref-based, by the way? We have too many prefs as it is, we > shouldn't be adding more. In fact the entire "Appearance" pref panel should be > removed (along with all the prefs on that panel), not bloated further. Couldn't we just base the pref on whether or not the user left the previous oversized image at its normal size or not and not have any UI for it? This same idea has been discussed in the bug for auto-hiding the sidebar.
I'm sorry but I can't believe anyone could say there are too many preferences. in any given program I think there are far too few user preferences. why would you *want* to be restricted to what the developer likes? I think this should definitely be a preference.
> In fact the entire "Appearance" pref panel should be removed Certainly not "should" !!! Perhaps "could". But I **STRONGLY** disagree with that.
i think is useful to have more feedback when the image was automatically resized, some icon or message in the status bar can do the job
I noticed this is checked in on the trunk, so i played around with it. My only complain is that there is no way to change this setting on the fly, so you have to go and change it in prefs. Therefore, if I have it disabled by default, it won't preserve when I turn it off for an image, so if I'm viewing a group, I will have to either manually resize them or go into Edit -> Prefs which is kind of a pain. There must be a better way. keithkml: Hixie was just complaining about yet another pref with UI. + <menuitem id="context-fitimage" + type="checkbox" + label="&fitImageCmd.label;" + accesskey="&fitImageCmd.accesskey;" + oncommand="gContextMenu.toggleImageSize();"/> + + <checkbox id="enableAutomaticImageResizing" + label="&enableAutomaticImageResizing.label;" + accesskey="&enableAutomaticImageResizing.accesskey;" + prefstring="browser.enable_automatic_image_resizing"/> (from the patch) Since these UI items, the menu and the prefs box in appearance are in the trunk, I wouldn't get concerned unless there was a concensus among developers it should be changed.
Also, note that when you turn off auto-resizing, you can't resize an image at all. Patch for all this is on its way.
Flags: blocking1.3b? → blocking1.3b-
Attached patch patch (obsolete) (deleted) — — Splinter Review
Stab at a patch to fix said issues. Works for me. You can install it after the other two patches above.
Attachment #112504 - Flags: review?(caillon-obsolete)
I just wanted to mention that you have to use Jan's patches before mine. Since her patches are already on the trunk, you don't have to install them if your tree is up to date. I used Jan's framework and extended/modified it a bit. Thanks Jan for this nice feature. Added: You can now manually resize when you have it set not to auto-resize. It won't affect the preference. There is a remember pref to allow Mozilla to auto-resize only if you shrunk image last time (Good for going through a list of images). These prefs are hooked up to 3 radio buttons in a group in the prefs window, everything still fits nicely There is a hidden pref which remembers what you did to the last image (shrunk or not)
Comment on attachment 112504 [details] [diff] [review] patch >+<groupbox id="imageResizeOptions"> >+ <caption label="&automaticImageResizeOptions.label;"/> >+ <radiogroup id="automaticImageResizeMethod" align="start" >+ prefstring="browser.automatic_image_resizing"> >+ <radio group="automaticImageResizeMethod" value="1" label="&autoResizeRadio.label;" accesskey="&autoResizeRadio.accesskey;"/> >+ <radio group="automaticImageResizeMethod" value="2" label="&lastChoiceRadio.label;" accesskey="&lastChoiceRadio.accesskey;"/> >+ <radio group="automaticImageResizeMethod" value="0" label="&noResizeRadio.label;" accesskey="&noResizeRadio.accesskey;"/> max 80 chars per line please. >+<!ENTITY automaticImageResizeOptions.label "Resize larger images than the browser window"> Same thing here. Please check all your code additions, you often go far beyond that 80 chars limit and it makes your source harder to read and manipulate. >- PRPackedBool mImageResizingEnabled; >- PRPackedBool mImageIsOverflowing; >- PRPackedBool mImageIsResized; >+ PRInt32 mImageResizingMethod; // How to perform auto-zoom 0=none 1=auto 2=based on last images >+ PRPackedBool mImageResizingLast; // Was the image successfully shrunk before? >+ PRPackedBool mImageShouldResize; // A combination of mImageResizingLast and mImageResizingMethod >+ PRPackedBool mImageIsOverflowing; // Does the image overflow the window? >+ PRPackedBool mImageIsResized; // Whether or not image is shrunken Just move the PRInt32 before or after the all the PRPackedBool ? >+ mImageShouldResize = PR_FALSE; >+ if (mImageResizingMethod == 1) mImageShouldResize = PR_TRUE; This last test should stand on 2 lines, not one. >- if (mImageResizingEnabled) { >- nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mImageElement); >- target->RemoveEventListener(NS_LITERAL_STRING("click"), this, PR_FALSE); >- >- target = do_QueryInterface(mScriptGlobalObject); >- target->RemoveEventListener(NS_LITERAL_STRING("resize"), this, PR_FALSE); >- target->RemoveEventListener(NS_LITERAL_STRING("keypress"), this, PR_FALSE); >- } >+ target->RemoveEventListener(NS_LITERAL_STRING("click"), this, PR_FALSE); >+ target = do_QueryInterface(mScriptGlobalObject); >+ target->RemoveEventListener(NS_LITERAL_STRING("resize"), this, PR_FALSE); >+ target->RemoveEventListener(NS_LITERAL_STRING("keypress"), this, PR_FALSE); > } > else { >- if (mImageResizingEnabled) { >- nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mImageElement); >- target->AddEventListener(NS_LITERAL_STRING("click"), this, PR_FALSE); >- >- target = do_QueryInterface(aScriptGlobalObject); >- target->AddEventListener(NS_LITERAL_STRING("resize"), this, PR_FALSE); >- target->AddEventListener(NS_LITERAL_STRING("keypress"), this, PR_FALSE); >- } >+ target->AddEventListener(NS_LITERAL_STRING("click"), this, PR_FALSE); >+ target = do_QueryInterface(aScriptGlobalObject); >+ target->AddEventListener(NS_LITERAL_STRING("resize"), this, PR_FALSE); >+ target->AddEventListener(NS_LITERAL_STRING("keypress"), this, PR_FALSE); > } You could use a few NS_NAMED_LITERAL_STRING here. >- mImageElement->SetAttribute(NS_LITERAL_STRING("style"), >- NS_LITERAL_STRING("cursor: move")); >+ mImageElement->SetAttribute(NS_LITERAL_STRING("style"), >+ NS_LITERAL_STRING("cursor: move")); Hmmm. Call me a maniac but I'd like to see another approach: you put a new style rule for a given class in the stylesheet applied to the image document and you set/reset the class on mImageElement instead of setting/removing the 'cursor' property (btw, you do that now through 'style' attribute and that is more expensive than a CSS OM call for the single 'cursor' property). That way, you can easily extend the styles applied to a resized image if you need it, or if an emebeddor needs it. You can even attach an XBL behavior that way and that's wort making the change, imho. In particular if we ever want to extend the image view and put a special menu or whatever. >+ mImageElement->RemoveAttribute(NS_LITERAL_STRING("width")); Just wondering why you apply/remove a WIDTH attribute instead of the CSS 'width' property. >+ if (!mImageIsOverflowing) { >+ mImageElement->RemoveAttribute(NS_LITERAL_STRING("style")); See above. >+ if (mImageIsOverflowing) >+ mImageElement->SetAttribute(NS_LITERAL_STRING("style"), >+ NS_LITERAL_STRING("cursor: move")); See above. > nsAutoString eventType; > aEvent->GetType(eventType); >- if (eventType.Equals(NS_LITERAL_STRING("resize"))) { >- CheckOverflowing(); >- } >- else if (eventType.Equals(NS_LITERAL_STRING("click"))) { >- ToggleImageSize(); >- } >+ if (eventType.Equals(NS_LITERAL_STRING("resize")) || >+ eventType.Equals(NS_LITERAL_STRING("click"))) >+ ToggleImageSize(); > else if (eventType.Equals(NS_LITERAL_STRING("keypress"))) { Named literal strings again ?
Attachment #112504 - Flags: review?(caillon-obsolete) → review-
Comment on attachment 112504 [details] [diff] [review] patch > Hmmm. Call me a maniac but I'd like to see another approach: you put > a new style rule for a given class in the stylesheet applied to the > image document and you set/reset the class on mImageElement instead > of setting/removing the 'cursor' property I just realize that we don't apply a special stylesheet when only an image is viewed in the browser.... If you don't want to dive into that, fix the minor things I commented and you have my r=.
Attachment #112504 - Flags: review- → review+
Comment on attachment 112504 [details] [diff] [review] patch > Hmmm. Call me a maniac but I'd like to see another approach: you put > a new style rule for a given class in the stylesheet applied to the > image document and you set/reset the class on mImageElement instead > of setting/removing the 'cursor' property I just realize that we don't apply a special stylesheet when only an image is viewed in the browser.... If you don't want to dive into that, fix the minor things I commented and you have my r=.
Depends on: 189982
Moved to bug 190545 per Jan's suggestion. This bug is getting too big and its technically fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
No longer depends on: 189982
Resolution: --- → FIXED
Comment on attachment 112504 [details] [diff] [review] patch This patch is being moved to bug 190545
Attachment #112504 - Attachment is obsolete: true
VERIFIED FIXED. Jan's patches are in the trunk, and haven't caused any problems.
Status: RESOLVED → VERIFIED
They added a pref, which _is_ a problem.
<quote> ------- Additional Comment #157 From Christian Biesinger 2003-01-20 00:03 ------- Mozilla has no image toolbar, and images don't have meta tags. </quote> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnie60/html/whatsnewpublicpreview.asp Microsoft has a meta tag for disabling Internet Explorer's image toolbar (but not automatic image resizing?). For automatic image zooming, perhaps Mozilla should have a meta tag for developers to enable or disable this.
> perhaps Mozilla should have a meta tag Mozilla only allows zooming images that are being viewed standalone (which you know if you read this bug and the patches). Images in HTML documents are not zoomable, unlike IE. Now where do you propose to put this meta tag?
see, that's for in-page images. these images don't support auto-resizing anyway. and I really hope we never implement the image toolbar that could be disabled via that meta tag; but if we did, I hope that we would not support the meta tag, because the browser belongs to the user, not the web developer.
*** Bug 190667 has been marked as a duplicate of this bug. ***
"see, that's for in-page images. these images don't support auto-resizing anyway." Addressing the (rhetorical?) question in Comment #179, a user might encounter an HTML page containing a large inline image (for example, a screenshot) and not want to scroll horizontally. "these images" could use a separate bug, though i doubt that many users really need this functionality; plus, this would burden developers and users. Separately, i did not suggest adding an image toolbar, "read ... the patches", or read the entirety of this (corpulent) bug.
Nguyen: You wrote: ------- Additional Comment #155 From Nguyen, Alexander 2003-01-19 19:21 ------- Microsoft has a meta tag for IE's Image Toolbar (and "Smart"Tags). Should Mozilla follow the meta tag(s)? this does sound like suggesting that mozilla supports that toolbar, what else should it follow the meta tags for? and ANYWAY, that does _so_ not belong into this bug. or in a bug at all. for the large-image-in-page issue, you might want to file a separate bug; the meta tag stuff should probably be taken to direct email or stopped.
I can verify this is working in the Win32 (2003-01-28-04) trunk and OS X (2003-01-28-03). Under the OS X build, I noticed clicking on the image displays a 'Fit in Window' menu item listed in the contextual menu. This same menu item is missing from the contextual menu displayed in the windows build. I will file a bug on this problem.
on linux with build 2003012602+Xft, sometimes when rendering large images live (while updating the screen) as they download produces a black line at the bottom of each update that stays until i resize the browser window. sorry that's confusing to read... I click a link to go to an image, and it is larger than my browser window, and the autoresizing takes effect. I see the beveled outline of the image, as it has not started loading yet. then, from the top, lines begin to fill in, as the image is downloaded. mozilla updates the display every few seconds, rendering 1-10 new lines of the image at each update. however, at the bottom of the last line rendered during that update there is a black line with a height of one pixel. this would be fine if it were overwritten on the next update, but it is not; instead, the next update continues after the black line, so by the time the full image is rendered every 5 or 10 lines in the image is a black line. should this be a separate bug?
I think that's bug 152005 ...
Chris: Before you file a bug, I do see that "Fit to window" entry in the context menu on Windows. Are you sure you didn't have autoscaling disabled? In that case, it wouldn't appear (fixed with preliminary patch to bug 73322).
Chris: Sorry, I meant bug 190545. :-)
Why is this being done in the core? Why are we polluting Gecko with UI hacks? What happpened to glazman's XBL approach?
IIRC, glazman had to rewrite it in C++ too, because it was also required by an embeddor. Anyway, I can't imagine how I would implement it correctly in pure XBL. e.g. I needed to be notified that imagalib got the size of the image and then shrink the image.
> because it was also required by an embeddor And that's a reason for saddling _all_ embeddors with it?
Hmm, actually I don't know how many embeddors would like to use it at the moment and how many might want to use it in the future. We always have a possibility to build this code optionally (if we add ifdefs, etc.) The patch for this feature was not so big, it also cleaned a lot of code in nsImageDocument. On other hand, according to my perf measurements of toolbar grippies, XBL is the primary cause of slowdown.
I don't care so much about the footprint. The issue is with making core Gecko code more complex to support user interface features that could be done outside Gecko. Also, doing this outside Gecko would be a lot more flexible than baking it into Gecko. XBL performance is not a problem because we don't have a lot of benchmarks requiring us to load single image documents fast. Non-XUL embedders should just be able to override the image MIME types and use their own viewers for those types, instead of asking Gecko to handle them, if they want something more clever than a basic document containing the image. If they're not including XUL they shouldn't be asking Gecko to do UI for them.
> Why is this being done in the core? Why are we polluting Gecko with UI hacks? Because major embeddors don't have a JS/XBL/XUL layer above the editor and the browser. And if you consider it's not a UI thing but a core feature of an image/* browser... > What happpened to glazman's XBL approach? The patch is in the bug. It won't be in the trunk. I rewrote it in c++. > And that's a reason for saddling _all_ embeddors with it? Don't ask me. I'm only the software developer here.
*** Bug 190667 has been marked as a duplicate of this bug. ***
Those "major embedders" should have kept this feature in their own code instead of dumping it here.
Well, I like the feature itself and therefore am happy that it's in the mozilla tree. I can't speak about the right place in the code though. But it wasn't dumped here, the bug has been opened almost two years ago and been readable for everyone.
Arthur, there was a 5-day period between the code being attached and checked in. The two years previous did not affect the technical aspects of that code in _any_ way (they got completely ignored, basically). ;)
*** Bug 190667 has been marked as a duplicate of this bug. ***
Interesting feature. Leave it in. Since its a preference, those who want it and those who don't only have to set one option. Also, as for the comment about how a user would know to click the image to see the original images size, why not include a comment that says "Click the button to see the full sized image" and a button below it that says "Zoom In" and then a button on the larger one below it that says "Zoom Out".
I haven't read all the discussion, but this doesn't seem to work when the image is in a frame. Shouldn't this bug be reopened?
No, file a bug on that as blocking bug 190545.
Product: Core → Mozilla Application Suite
Blocks: 408392
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: