Open Bug 136382 Opened 24 years ago Updated 3 years ago

pref to hide placeholders while loading images

Categories

(Core :: Layout: Images, Video, and HTML Frames, enhancement, P5)

Product:
Core
Component:
Layout: Images, Video, and HTML Frames
Type:
enhancement

Tracking

()

Milestone:
Future

People

(Reporter: voidcartman, Unassigned)

References

Details

(Whiteboard: [WGATE])

Attachments

(2 obsolete files)

There should be an ui to disable to show image placeholders while loading images .
taking. easy fix I suppose.
Assignee: pavlov → cbiesinger
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch patch, backend (obsolete) — — Splinter Review
adds a new hidden preference browser.display.show_image_placeholders, defaulting to true; also adds this setting to all.js This is a |cvs diff -wu|
Attached patch working patch (obsolete) — — Splinter Review
this time containing all changed files
Attachment #78353 - Attachment is obsolete: true
Keywords: patch
OS: Linux → All
Hardware: PC → All
Worldgate and others would want this too; it was on my list.
Whiteboard: [WGATE]
Comment on attachment 78354 [details] [diff] [review] working patch r=rjesup@wgate.com. Please correct indentation in the first change section where you added an "if (dispicon){ }"
Attachment #78354 - Flags: review+
rjesup, I will. When I said this is a |cvs diff -w|, I meant that this way of creating the diff ignores whitespace changes - the real changes could easily get lost in the whitespace changes else, I thought. tor, could you super-review?
Status: NEW → ASSIGNED
Keywords: review
Summary: ui to disable image placeholders → preference to disable image placeholders
Target Milestone: --- → mozilla1.0
Comment on attachment 78354 [details] [diff] [review] working patch sr=tor
Attachment #78354 - Flags: superreview+
I didn't notice it was diff -w, sorry.
So we are getting an option in the ui too?
there's no ui yet. I'll work on that when the backend part is checked in.
patch checked in on trunk
Comment on attachment 78354 [details] [diff] [review] working patch a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #78354 - Flags: approval+
Comment on attachment 78354 [details] [diff] [review] working patch checked in on branch leaving bug open for ui
Attachment #78354 - Attachment is obsolete: true
Component: ImageLib → Image: Layout
adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword with verified1.0.0.
Keywords: fixed1.0.0
*** Bug 136711 has been marked as a duplicate of this bug. ***
Summary: preference to disable image placeholders → pref to hide placeholders while loading images
Why do we show the placeholder borders at all? If we fix bug 11011 then all the code for this bug would be redundant and we could just pick the best behaviour in html.css and let advanced users override it in userContent.css.
QA Contact: tpreston → ian
In fact, this entire pref should be removed and our behaviour hard coded (by using bug 11011) to borderless placeholders, as it says in our spec: http://www.hixie.ch/specs/alttext
Priority: -- → P5
Target Milestone: mozilla1.0 → Future
The availability of the pref to disable placeholders makes it easy to measure overhead with and without placeholder icons. The overhead is non-trivial. Ref most recent comments to: http://bugzilla.mozilla.org/show_bug.cgi?id=144533 Problem, I think, in current code is disable placeholders also disables broken-image icons. Broken icon have much more utility than transient placeholder icon. Given my personalopinions and value-system... given low-utility at non-trivial performance cost... I think default (by preference or hardcoding) should be disable placeholders, but enable broken icons.
>In fact, this entire pref should be removed and our >behaviour hard coded (by using bug 11011) to borderless >placeholders, as it says in our spec: Maybe you missed the line above: >Worldgate and others would want this too; it was on my list. Specs evolve to meet customer requirements, don't you think?
Sam, I implemented this pref like the reporter wanted it. If you want, I can replace it with two seperate prefs for the loading and broken placeholders.
Keywords: patch, review
Thanks! I think something like: - if (dispIcon) { + if ((dispIcon && (aIconId == NS_ICON_LOADING_IMAGE)) || + (brokIcon && (aIconId == NS_ICON_BROKEN_IMAGE))) { would be useful additional enhancement. Now investigating this hour overhead of PaintBorder. Since we're pref'ing this to death anyway :-) maybe a third pref for that?
Thanks! I think something like: - if (dispIcon) { + if ((dispIcon && (aIconId == NS_ICON_LOADING_IMAGE)) || + (brokIcon && (aIconId == NS_ICON_BROKEN_IMAGE))) { would be useful additional enhancement. Now investigating this hour overhead of PaintBorder. Since we're pref'ing this to death anyway :-) maybe a third pref for that?
Guys, guys. No prefs. We don't need prefs. Prefs are bloat. No need for prefs. Unless there is a very specific need for behaviour other than described in Mozilla's alt text spec [1], that is all that should be implemented. Is there such a need? [1] http://www.hixie.ch/specs/alttext
The need has been already described, I think. Customer need "Worldgate and others" and newly understood and described performance need "15% faster display 200 gif icons". Prefs not 'bloat'. If somebody ever got task to docuement all of them... that'd be expensive... but one-time check of prefs string, set packed boolean, runtime check that, is cheap, IMO.
I quickly coded up as 3 prefs, that is, enable/disable for each of: placeholders, broken-icons, and icon borders. There seems to be very little additional performance benefit from disable icon borders. Okay off I go for long weekend. Thanks guys!
I think an ui for it would be useful otherwise testers/customers cant easily figure out a preference like this actually exists.
Can someone please explain the need for these prefs. ``Customer need "Worldgate and others"'' means nothing to me.
Looks like we need to disable the loading_image_icon because it costs lots of performance (for certain systems?). Those who need the performance should be able to disable the icon and those who can live with slower page load can turn it on.
I'm having great difficulty believing that the image icons have such an effect on performance that we need a pref to disable them. If they do, then that's a bug in its own right, but should not be worked around by a pref.
Ian, I'm half inclined to agree, other side of coin, placeholder-icons are entirely useless baggage and should be removed altogther, no pref is needed, just yank it out.
Well, if you have a loading icon, when an image is found to be broken, the icon can change without moving the alt text around the box. But I would not really be against removing them from the placeholder boxes while the images are loading. mkaply, dbaron? Comments? (For background see comment 16 and thereabouts.)
Keywords: fixed1.0.0
For exceedingly rare case, if broken and alt-text, why is it a concern if alt-text moves or not? I just need help please understandding the basis of this concern.
I don't understand what you mean by "broken and alt text". Most <img> elements will have alt text, so if they are broken (404-missing, corrupted, unsupported format) then they will be broken and have alt text.
WRT prevelence of ALT= text, I stand corrected. Thanks! Where are we on this? I have my private code/build, implemented: pref("browser.display.show_image_placeholders", false); pref("browser.display.show_image_iconsborders", false); pref("browser.display.show_image_alttext", false); pref("browser.display.show_image_brokenicons", true); The 'alttext' pref control ALT= display during load, the ALT= text displayed if broken icon and 'brokenicons' true. It's not big or difficult code changes, I don't think my code/patch help much, its more matter of deciding to do it? My un-expert 'favorite behavior' is noted in prefs above (display nothing if not broken), but recognize other opinions equally valid. What can be done to reach closure on this?
Waitasec, WTF do we need the alt text pref for? Alt text should always be displayed if the image is broken or blocked, since that what's it was created for. Also, what is the iconsborders pref for?
I believe there should be no prefs, and the behaviour should be as described in http://www.hixie.ch/specs/alttext
I think I have not adequately explained/supported the larger goal/objective here. Ivan Eisen and I working comparison Mozilla to IE performance. We note Mozilla competitive in many/most our 'atomic' tests, these tests mostly simple 'flat' documents focused on one sort of content or another (i.e. a large text test, a forms test, a jpg test, a css table test, etc). Another set of tests we have is 'company' tests, locally hosted 'copy' of real corporate company web sites. We note that IE soundly whips Mozilla on performance of some of these tests. So we wonder why, if competitive on atomics, why trounced on 'real site' contnet. Analysis shows degree Mozilla loses related to number of objects forming site content. Further, not due to read/parse/render of content, but rather 'object level' overheads. Overheads with status messages and progress calculations for each url/object noted bugzilla 144533. Patch created and proposed patch for that. Next noted was this, placeholders, work done to produce transient content usually quickly replaced by actual content. With private fixes for these several things, our private Mozilla builds have shown significant improvement in the 'company' tests, validating the analysis about the overheads. The arguement against the analysis findings and proposed fixes would be the test environment, the local hosting of the pages, the high-speed host-client connection, exagerates the problem with the overheads. No arguement that, except to say we think this environment important to our customer set. Our customer set mostly corp business users and web-protocols company apps, not home-pc consumers web browsing. In corp environments, local app hosting and high-speed networks are typical. Hope this helps!
>From Christian Biesinger -Waitasec, WTF do we need the alt >text pref for? Alt text should always be displayed if the >image is broken or blocked, since that what's it was >created for. Currently, Alt text is displayed during load with placeholder. Pref would enable/disable alt text during load >Also, what is the iconsborders pref for? This is for enable/disable of transient frame border currently displayed during image load (placeholder icon and alt text formatted and displayed inside this frame border). Current code: the border, the placeholder icon, the alt-text are first formatted and drawn then overwritten by actual image when it arrives. Proposal would be not do this interim/transient work. If broken, then frame/placeholder/text replaced by frame/broken-icon/text. Propose no change this behavior.
If it's slow, make it fast. Don't remove it. Running Mozilla through a profiler like Quantify is the only real way to discover what the root cause of Mozilla's slowness is. If you read the spec I've quoted above, you'll see that Mozilla wants to have no prefs for this, and that the behaviour should be, in your terms: no border icon and alt-text are drawn while image is being downloaded if broken, then the icon is changed There should be no border. The alt text must be visible during download.
Ian, I've read the doc you have ref'd numerous times. I don't know what it adds for you to just ref it again. Both the doc and your posts are thin on motivation, that is, why the behavior should be the way you think it should be. Perhaps you could elaborate on that. FWIW, as comparison base, IE does not behave the way you describe.In some cases this 'extra work' that Mozilla does hurts performance in comparison to IE. Personally, I find the momentary flash of placeholders and alt-texts to be distracting, so obviously I'm missing the percieved user value. So if you could help me understand your obviously strong opinions about this...
The desired behavior has been debated to death in other bugs. We don't need to do it again. Please.
I apologize for not having that prior knowledge and Bugzilla search 'placeholder' and 'alt text' not revealing. Can you give ref these bugs please? Thanks.
Okay, read that - all or mostly about broken images! This isn't about that, its about pending images. http://www.pythonet.org/the-spanish-inquisition.html
Just a ref to a page having pending and broken of course.
A "pending" image is just a "broken" image whose "broken" status is that it hasn't been downloaded yet. If you really want motivation: no border: because there is no point in having a border. icon and alt-text are drawn while image is being downloaded: so that the user can see what images are still pending and can see what they represent (the text) and can right click on them (the icon). if broken, then the icon is changed: see the bugs dbaron mentioned for motivation for broken images. The alt text must be visible during download because there is no guarentee that the image won't take minutes to download, and while it is downloading, the page should convey the meaning of the image through alternate text so that the user doesn't miss out on the meaning, waiting for the image. Is that enough?
>A "pending" image is just a "broken" image whose "broken" >status is that it hasn't been downloaded yet. Hummm... well, accept that as loose definition, its certainly not anyway near the way the code for this actually works. Ian, I'm looking code cogitating if/how we can get win-win compromise. I'd suggest most performance concern this is with SMALL images - usually buttons, arrows, various little screen widgets. Its these kind of gifs, typically a few hundred bytes size, where image object management overhead, including placeholders dominates the performance. So another approach would be, for images smaller than X pixels in height/width, then eliminate placeholder & alttext processing during load. What you think about this? >The alt text must be visible during download because there >is no guarentee that the image won't take minutes to >download, and while..... That's the rub. Here, in layout, the image stream source, and speed of that source, is not known, Image might be coming from 14.4 kbps modem or local disk cache. Might take milliseconds or minutes. In the minutes case, the user feedback is valuable, in the milliseconds case its unneeded overhead. (both processor overhead and visual overhead). I still looking if there's a solution to better behavior based on image stream source (that is, maybe, not do placeholders if image source is file stream). Anybody thoughts on that?
> I'd suggest most performance concern this is with SMALL images It should be taking a trivial amount of time to draw the 16x16 icon. If it isn't, that's a _bug_ and should be _fixed_, not wall-papered over.
Ian, I will try again. Using very short sentences. I hope this helps. Gif image processing on Mozilla is not slow. When the html says draw one small gif -- then IE draws one small gif. When the html says draw one small gif -- then Mozilla draws two small gif's. For one small gif the time for either IE or Mozilla is trivial. When the html says draw 100 small gif's -- then IE draws 100 small gif's. When the html says draw 100 small gif -- then Mozilla draws 200 small gif's. Trivial wasted time, done many times, becomes non-trivial. If IE-like performance is desirable, --then Mozilla cannot do twice the work of IE. I hope this is clearer to you.
What makes you think that the performance cost is the drawing of the actual placeholder images?
David, on my own build I've coded up prefs for placeholder icons off/on, borders off/on, alt-text off/on, and measured the combinations. Borders are cheapest (just line draws, not bit-blt). Text workload 'depends', The placeholders consistent expensive. Turn all off and get close to IE. But alt-text seems to have value outweighing perf cost, IMO. Again, I'd like reach some code-workable compromise with those having strong best-UI-design opinions. My strong opinion is, of course, best-performance.
Could we see the numbers from your tests, and some details about what sort of pages and network setup you used for them? Thanks.
You should not be able to measure the difference between us drawing 100 small GIFs and us drawing 100 small GIFs plus 100 placeholder GIFs. If you can, that's a bug. It doesn't mean we should stop drawing the GIFs altogether.
Hixie, I think what emrick is trying to tell you is that the performance cost is coming not from the actual drawing of the little insignifigant placeholder images, but rather from the object-creation and object-deletion overhead involved in inserting the placeholder into the document and then deleting it again before replacing it with the real image. That overhead is the same regardless of the size of the placeholder image, and the same regardless of the speed of actually loading or displaying the images.
That overhead should most _definitely_ not take a measurable amount of time.
I agree with the view that displaying borders, image icons and alt text is pretty useless when the download is racing by. When it is useful is when things are slow. Perhaps instead of a yes/no for displaying these things it should be hooked up to a timer, so that one could decide how many milliseconds after the image load request is sent that these things should be displayed. '0' would be the current behavior, '-1' be the equivalent of no, a good default might be say 2000 (for 2 seconds) or something in that territory. There's little point to drawing everything only to have it covered up by the loaded image so quickly it served no purpose. However, if things are loading very slowly, knowing that an image of a given size with the given description will be loaded in that blank space can be useful. This way, the only time you take the performance penalty is when you are waiting on the network anyway, when it doesn't matter, but when the information conveyed by the placeholders is most needed.
bugs w/o patches -> NEW
Status: ASSIGNED → NEW
.
Assignee: cbiesinger → pavlov
QA Contact: ian → tpreston
without placeholder pages looks nicest! specialy pages with many images spacers like www.microsoft.com:))) the sooner this bug is sovled the better it will be.
it would be very nice to have an option for turning on image placeholders only if images are not loaded. for example if one is using a mobile phone connection to the internet one turns off image display to save loading time. it would be nice if the image box (if width and height is in the <img > tag) draw the size of the image. also it would be very nice if the title or alt displayed if the image was not loaded.
k b, alt not showing if images are disabled is a bug (which I thought was fixed), but it's another one. so please test a current nightly, search for existing bugs, and file a new bug if you still see that and no bug is filed yet.
At this point, we load images from content, so the placeholder perf issue should be nearly moot... What is the purpose in life of this bug? We have a pref to not draw the placeholders, no?
Sam, what are your measurements showing in current builds?
Sorry, I was re-assigned to another project at the beginning of the year, and no longer working on Mozilla.
Sam, is Ivan still working on Mozilla? Is he cced on this bug?
No, sorry, Ivan is doing other work too.
(In reply to comment #62) > At this point, we load images from content, so the placeholder perf issue should > be nearly moot... > > What is the purpose in life of this bug? We have a pref to not draw the > placeholders, no? The purpose of this bug is that disabling the pref still causes a placeholder-like image to be drawn until the actual image is downloaded and displayed in the browser.
Placeholders are not shown although browser.display.show_image_placeholders is set true. This is a bug. Consider a html page containing a dozend images with descriptions, for example http://www.icmag.com/ic/showthread.php?t=26180&page=1&pp=15 The connection is slow hence one reads the descriptions and selects an image for viewing. This is not possible with Mozilla 1.7.8 on Linux. Broken functionality like this is a PITA. I have used Netscape/Mozilla since 1997. Back then, placeholders did work.
That page is in standards mode, so the images are replaced with alt text. That is, the behavior on that page has nothing to do with this bug. Of course it's not really clear to me what this bug _is_ about at this point.
Assignee: pavlov → nobody
QA Contact: tpreston → layout.images
Product: Core → Core Graveyard
Product: Core Graveyard → Core
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: