WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
186614
Make it possible to add a border around loading or failed-to-load images
https://bugs.webkit.org/show_bug.cgi?id=186614
Summary
Make it possible to add a border around loading or failed-to-load images
Tim Horton
Reported
2018-06-13 18:22:02 PDT
Make it possible to add a border around loading or failed-to-load images
Attachments
Patch
(19.49 KB, patch)
2018-06-13 18:22 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-sierra
(805.41 KB, application/zip)
2018-06-13 18:59 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews116 for mac-sierra
(544.25 KB, application/zip)
2018-06-13 19:35 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews201 for win-future
(12.76 MB, application/zip)
2018-06-14 15:57 PDT
,
EWS Watchlist
no flags
Details
Patch
(20.31 KB, patch)
2018-06-21 00:01 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-sierra
(540.67 KB, application/zip)
2018-06-21 00:54 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews114 for mac-sierra
(537.97 KB, application/zip)
2018-06-21 01:05 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews106 for mac-sierra-wk2
(2.83 MB, application/zip)
2018-06-21 05:14 PDT
,
EWS Watchlist
no flags
Details
Patch
(20.28 KB, patch)
2018-06-22 13:34 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(23.05 KB, patch)
2018-06-22 16:24 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Tim Horton
Comment 1
2018-06-13 18:22:48 PDT
Created
attachment 342717
[details]
Patch
EWS Watchlist
Comment 2
2018-06-13 18:59:55 PDT
Comment on
attachment 342717
[details]
Patch
Attachment 342717
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/8171818
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 3
2018-06-13 18:59:56 PDT
Created
attachment 342718
[details]
Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
zalan
Comment 4
2018-06-13 19:32:55 PDT
Comment on
attachment 342717
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=342717&action=review
> Source/WebCore/rendering/RenderImage.cpp:381 > +void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const LayoutUnit& borderWidth)
void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, LayoutPoint paintOffset, LayoutUnit borderWidth)
> Source/WebCore/rendering/RenderImage.cpp:390 > + LayoutUnit leftBorder = borderLeft(); > + LayoutUnit topBorder = borderTop(); > + LayoutUnit leftPad = paddingLeft(); > + LayoutUnit topPad = paddingTop();
auto and please don't abbrev.
> Source/WebCore/rendering/RenderImage.cpp:480 > + LayoutUnit incompleteImageBorderWidth = LayoutUnit(2 / deviceScaleFactor);
This makes the image borders thicker. is it intentional?
> Source/WebCore/rendering/RenderImage.h:110 > + void paintIncompleteImageOutline(PaintInfo&, const LayoutPoint&, const LayoutUnit&);
shouldn't this paint function be const?
EWS Watchlist
Comment 5
2018-06-13 19:35:30 PDT
Comment on
attachment 342717
[details]
Patch
Attachment 342717
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/8171988
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 6
2018-06-13 19:35:31 PDT
Created
attachment 342719
[details]
Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Tim Horton
Comment 7
2018-06-13 21:06:25 PDT
(In reply to zalan from
comment #4
)
> Comment on
attachment 342717
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=342717&action=review
> > > Source/WebCore/rendering/RenderImage.cpp:381 > > +void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const LayoutUnit& borderWidth) > > void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, > LayoutPoint paintOffset, LayoutUnit borderWidth) > > > Source/WebCore/rendering/RenderImage.cpp:390 > > + LayoutUnit leftBorder = borderLeft(); > > + LayoutUnit topBorder = borderTop(); > > + LayoutUnit leftPad = paddingLeft(); > > + LayoutUnit topPad = paddingTop(); > > auto > and please don't abbrev.
Just moved, but fine :)
> > Source/WebCore/rendering/RenderImage.cpp:480 > > + LayoutUnit incompleteImageBorderWidth = LayoutUnit(2 / deviceScaleFactor); > > This makes the image borders thicker. is it intentional?
Technically intentional (see radar), but it /is/ kind of weird that the incomplete border is thicker than the failed-to-load border. Maybe we should change both. Or neither.
> > Source/WebCore/rendering/RenderImage.h:110 > > + void paintIncompleteImageOutline(PaintInfo&, const LayoutPoint&, const LayoutUnit&); > > shouldn't this paint function be const?
Almost certainly.
Tim Horton
Comment 8
2018-06-13 21:08:24 PDT
It sort of looks like the tests got off-by-one?
EWS Watchlist
Comment 9
2018-06-14 15:57:12 PDT
Comment on
attachment 342717
[details]
Patch
Attachment 342717
[details]
did not pass win-ews (win): Output:
http://webkit-queues.webkit.org/results/8185401
New failing tests: http/tests/images/loading-image-border.html
EWS Watchlist
Comment 10
2018-06-14 15:57:23 PDT
Created
attachment 342771
[details]
Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Tim Horton
Comment 11
2018-06-21 00:01:08 PDT
Created
attachment 343218
[details]
Patch
EWS Watchlist
Comment 12
2018-06-21 00:54:26 PDT
Comment on
attachment 343218
[details]
Patch
Attachment 343218
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/8274643
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 13
2018-06-21 00:54:28 PDT
Created
attachment 343222
[details]
Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
EWS Watchlist
Comment 14
2018-06-21 01:05:14 PDT
Comment on
attachment 343218
[details]
Patch
Attachment 343218
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/8274630
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 15
2018-06-21 01:05:16 PDT
Created
attachment 343223
[details]
Archive of layout-test-results from ews114 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6
EWS Watchlist
Comment 16
2018-06-21 05:13:58 PDT
Comment on
attachment 343218
[details]
Patch
Attachment 343218
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/8276262
New failing tests: imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html
EWS Watchlist
Comment 17
2018-06-21 05:14:00 PDT
Created
attachment 343237
[details]
Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Wenson Hsieh
Comment 18
2018-06-21 07:35:47 PDT
Comment on
attachment 343218
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=343218&action=review
> Source/WebCore/page/Settings.yaml:755 > + initial: false
Should we trigger a repaint here when the value changes? (Probably not that important though, since our client is only going to enable this from the get-go and not fiddle with it later.)
Anders Carlsson
Comment 19
2018-06-21 08:11:33 PDT
Comment on
attachment 343218
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=343218&action=review
> Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm:1266 > +- (void)_setIncompleteImageBorderEnabled:(BOOL)incompleteImageBorderEnabled > +{ > + _preferences->setIncompleteImageBorderEnabled(incompleteImageBorderEnabled); > +} > + > +- (BOOL)_incompleteImageBorderEnabled > +{ > + return _preferences->incompleteImageBorderEnabled(); > +}
Doesn't seem like this should be a preference.
zalan
Comment 20
2018-06-21 08:13:44 PDT
Comment on
attachment 343218
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=343218&action=review
> Source/WebCore/rendering/RenderImage.cpp:382 > +void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const LayoutUnit& borderWidth) const
Apparently you missed it from my previous review. const LayoutPoint& paintOffset -> LayoutPoint const LayoutUnit& borderWidth -> LayoutUnit
> Source/WebCore/rendering/RenderImage.cpp:384 > + LayoutSize contentSize = this->contentSize();
auto
Tim Horton
Comment 21
2018-06-22 13:34:46 PDT
Created
attachment 343361
[details]
Patch
Tim Horton
Comment 22
2018-06-22 16:24:41 PDT
Created
attachment 343392
[details]
Patch
WebKit Commit Bot
Comment 23
2018-06-22 17:53:03 PDT
Comment on
attachment 343392
[details]
Patch Clearing flags on attachment: 343392 Committed
r233115
: <
https://trac.webkit.org/changeset/233115
>
WebKit Commit Bot
Comment 24
2018-06-22 17:53:04 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 25
2018-06-22 17:55:11 PDT
<
rdar://problem/41388828
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug