Open Bug 136382 Opened 22 years ago Updated 2 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: