Bug 15210 - Draw image outline for broken images
Summary: Draw image outline for broken images
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
Keywords: HasReduction
Depends on:
Reported: 2007-09-13 17:39 PDT by Peter Kasting
Modified: 2007-10-14 05:09 PDT (History)
0 users

See Also:

patch v1 (1.62 KB, patch)
2007-09-13 18:11 PDT, Peter Kasting
hyatt: review-
Details | Formatted Diff | Diff
patch v2 (2.18 KB, patch)
2007-09-14 13:44 PDT, Peter Kasting
mjs: review+
Details | Formatted Diff | Diff
Test case (broken image) (45 bytes, text/html)
2007-09-18 07:50 PDT, David Kilzer (:ddkilzer)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2007-09-13 17:39:14 PDT
In Firefox, a broken image is displayed as a hollow outline the size of the image (when known), with a broken image icon in the upper left.  In Safari, a broken image is displayed as an icon centered where the image would go, but with no border.  The lack of a border can make it harder to figure out what images on a page, exactly, are broken, and with very large images means it's easy to miss the icon entirely.  It would be nice to extend RenderImage.cpp to show a border in this case.
Comment 1 Peter Kasting 2007-09-13 18:11:40 PDT
Created attachment 16286 [details]
patch v1

Simple patch.  Some archaeology in trac tells me that this code dates from almost the very beginning of WebCore, back when all the commits were huge "land all the changes in SNAZZY_WEBKIT_BRANCH_3" affairs.
Comment 2 Dave Hyatt 2007-09-13 20:24:47 PDT
Comment on attachment 16286 [details]
patch v1

You need to adjust the usableWidth/Height to exclude the drawn outline, since the error image will potentially draw on top of the outline otherwise.

We'll need to discuss this change I think, since it constitutes a UI change to Safari.  It's definitely too late to make a change like this for Leopard, so it will have to wait for a bit.
Comment 3 Dave Hyatt 2007-09-13 20:26:19 PDT
alt text has the same bug (already).  That should be fixed too.

Comment 4 Peter Kasting 2007-09-13 22:44:58 PDT
Honestly it seems OK if the error image draws over the outline (as long as the two aren't crazily alpha-blended), but I'll take a look at making that change for both the broken image icon and the alt text in the next iteration of the patch.

Waiting for the trunk to go to some post-Leopard-branch state is fine with me, but I'm not totally clear on what the schedule for such things is.  Maybe there's already a wiki page explaining such things?
Comment 5 Peter Kasting 2007-09-14 13:44:04 PDT
Created attachment 16295 [details]
patch v2

This subtracts off the outline pixels when calculating the usable dimensions.

It might be good to go even further and subtract 4 in each direction instead of 2, to leave a 1 px buffer inside the outline.  Another possible change to this patch is that, when there's an error, we could avoid drawing the outline if the error image would fit without it but not with it (since if you have to pick one or the other, the error image is probably better than the outline).  Input welcome.
Comment 6 David Kilzer (:ddkilzer) 2007-09-18 07:50:29 PDT
Created attachment 16311 [details]
Test case (broken image)
Comment 7 Maciej Stachowiak 2007-09-29 19:34:13 PDT
I asked some of the more UI-oriented people on the Safari team what they think of this change.
Comment 8 Maciej Stachowiak 2007-09-29 23:25:58 PDT
Comment on attachment 16295 [details]
patch v2

r=me for feature-branch (or after feature-branch is merged back).
Comment 9 Mark Rowe (bdash) 2007-10-14 05:09:21 PDT
Landed in r26591.