Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(148)

Issue 696123002: Revert of Use Shadow DOM to display fallback content for images (Closed)

Created:
6 years, 1 month ago by rhogan
Modified:
6 years, 1 month ago
Reviewers:
esprehn
CC:
eseidel, jbroman, Julien - ping for review, ojan
Project:
blink
Visibility:
Public.

Description

Revert of Use Shadow DOM to display fallback content for images (patchset #28 id:540001 of https://codereview.chromium.org/481753002/) Reason for revert: Broke accessibility tests on Chrome build Original issue's description: > This replaces the use of painting in RenderImage to display an image's alt text > with an implementation in shadow DOM. This initial implementation is close in > appearance to the legacy display of alt-text but will ultimately move closer to > the one seen in Firefox and described in http://hixie.ch/specs/alttext. > > The alt-text and broken-image icon is now rendered as: > > <style> > #alttext-container { > overflow: hidden; > border: 1px solid silver; > padding: 1px; > display: inline-block; > } > #alttext { display: none; overflow: hidden;} > </style> > <div id="alttext-container"> > <img src="data:png, [broken-image-icon]" width="16" height="16" align="left" style="margin: 0px"> > <div id="alt-text">Alt text in here</div> > </div> > > Some notes on the way the fallback content is now rendered: > > - The fallback content is rendered inside an inline-block so it does not calculate > its dimensions the way a replaced element (i.e. an image) would (as defined by > http://www.w3.org/TR/CSS2/visudet.html#inline-replaced-width). So where one of width > or height is auto but the other is not, the fallback content will not exactly match > the dimensions calculated by RenderImage in error mode currently. We do a modest > imitation of the logic in quirks mode, but in strict mode it will behave exactly like an inline-block. > > - Where the image has no src attribute and no alt attribute RenderImage.cpp > still looks after the painting of the element - no fallback content is generated > and no broken image is displayed. This is consistent with existing behaviour. > > - The only image resource requests that still use the synchronous load path are > those where the item is cached (and didn't error out), where it's a data: source, > where its the fallback image for an object element, or where it's the image > for a main resource load. All other image loads are now asynchronous so that > fallback content can be loaded outside the style recalc phase. > > > - Instead of aborting an image resource request when the src element is empty > (i.e. src='') we now allow the request to go through so that it can fail and > invoke the fallback content in HTMLImageLoader.cpp. > > - As you can see in the new result for fast/borders/rtl-border-05.html, since > the alt-text is displayed as an inline-block it no longer artificially shrinks > any border on the element to the broken-image icon. > > Some notes on the rebaselined test results: > > - I've modified inspector/network/network-image-404.html to output the state of > both the resource requests, i.e. the failed one and the resource request for the > data:png to display the broken image icon. > > - I've added a missing support file for fast/css/counters/complex-before.html - > its absence meant that the result was polluted by the behaviour of broken image > rendering. > > - Likewise for fast/images/imagemap-polygon* tests - our new rendering of failed > image loads was interfering with an assumption in the tests that a broken image > still painted a RenderImage. So I've removed the src attribute to allow the > assumption hold (img elements without a src attribute are painted by > RenderImage). > > - http/tests/security/local-image-from-remote-whitelisted.html has been masking > a bug - blink does not load the image even though it is whitelisted. I am > rebaselining the test to reflect the failure revealed by this CL and tracking > a fix under crbug.com/410949. > > - I have altered fast/forms/state-restore-to-non-edited-controls.html to wait > 100ms before submitting the form as the image load in the input element is now > asynchronous. > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184770 TBR=esprehn@chromium.org NOTREECHECKS=true NOTRY=true Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184773

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -601 lines) Patch
M LayoutTests/TestExpectations View 2 chunks +1 line, -128 lines 0 comments Download
D LayoutTests/fast/css/counters/support/square-outline-32x32.png View Binary file 0 comments Download
M LayoutTests/fast/forms/state-restore-to-non-edited-controls.html View 1 chunk +1 line, -1 line 0 comments Download
D LayoutTests/fast/images/alt-text-wrapping.html View 1 chunk +0 lines, -1 line 0 comments Download
D LayoutTests/fast/images/alt-text-wrapping-expected.html View 1 chunk +0 lines, -17 lines 0 comments Download
M LayoutTests/fast/images/imagemap-circle-focus-ring.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-outline-color.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-zoom.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-zoom-style.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-focus-ring-zoom-style-expected.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-nested-area.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-overflowing-circle-focus-ring.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-overflowing-polygon-focus-ring.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/imagemap-polygon-focus-ring.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/inspector/network/network-image-404.html View 1 chunk +2 lines, -8 lines 0 comments Download
M LayoutTests/http/tests/misc/resources/generatedimage.php View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/accessibility/AXRenderObject.cpp View 1 chunk +0 lines, -5 lines 0 comments Download
M Source/core/core.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/editing/TextIterator.cpp View 4 chunks +3 lines, -6 lines 0 comments Download
M Source/core/html/HTMLElement.h View 1 chunk +0 lines, -4 lines 0 comments Download
M Source/core/html/HTMLImageElement.h View 5 chunks +1 line, -13 lines 0 comments Download
M Source/core/html/HTMLImageElement.cpp View 9 chunks +13 lines, -72 lines 0 comments Download
D Source/core/html/HTMLImageFallbackHelper.h View 1 chunk +0 lines, -39 lines 0 comments Download
D Source/core/html/HTMLImageFallbackHelper.cpp View 1 chunk +0 lines, -131 lines 0 comments Download
M Source/core/html/HTMLImageLoader.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/html/HTMLImageLoader.cpp View 2 chunks +4 lines, -13 lines 0 comments Download
M Source/core/html/HTMLInputElement.h View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/html/HTMLInputElement.cpp View 3 chunks +4 lines, -10 lines 0 comments Download
M Source/core/html/forms/BaseButtonInputType.h View 1 chunk +2 lines, -3 lines 0 comments Download
M Source/core/html/forms/ImageInputType.h View 3 chunks +0 lines, -9 lines 0 comments Download
M Source/core/html/forms/ImageInputType.cpp View 7 chunks +16 lines, -79 lines 0 comments Download
M Source/core/html/forms/InputTypeView.h View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/html/forms/InputTypeView.cpp View 1 chunk +0 lines, -8 lines 0 comments Download
M Source/core/loader/ImageLoader.h View 4 chunks +7 lines, -3 lines 0 comments Download
M Source/core/loader/ImageLoader.cpp View 5 chunks +13 lines, -20 lines 0 comments Download
M Source/core/paint/ImagePainter.cpp View 1 chunk +65 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderImage.h View 2 chunks +9 lines, -1 line 0 comments Download
M Source/core/rendering/RenderImage.cpp View 7 chunks +74 lines, -4 lines 0 comments Download
M Source/web/tests/ActivityLoggerTest.cpp View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
rhogan
Created Revert of Use Shadow DOM to display fallback content for images
6 years, 1 month ago (2014-11-01 15:05:59 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/696123002/1
6 years, 1 month ago (2014-11-01 15:06:22 UTC) #2
commit-bot: I haz the power
6 years, 1 month ago (2014-11-01 15:07:32 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 (id:1) as 184773

Powered by Google App Engine
This is Rietveld 408576698