Attachment 342717 Details for Bug 186614 – Patch

[patch] Patch

bug-186614-20180613182247.patch (text/plain), 19.49 KB, created by Tim Horton on 2018-06-13 18:22:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2018-06-13 18:22:48 PDT

Size: 19.49 KB

patch

obsolete

>Subversion Revision: 232770
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6fd2d905281a508ed0bf4e40b4b455aef727bb1a..f8e56e1423fa16222ac355ca69cd921a591e19bf 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-06-13  Tim Horton  <timothy_horton@apple.com>
>+
>+        Make it possible to add a border around loading or failed-to-load images
>+        https://bugs.webkit.org/show_bug.cgi?id=186614
>+        <rdar://problem/39050152>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: http/tests/images/loading-image-border.html
>+               http/tests/images/loading-image-no-border.html
>+
>+        
>+        * rendering/RenderImage.cpp:
>+        (WebCore::RenderImage::paintIncompleteImageOutline):
>+        (WebCore::RenderImage::paintReplaced):
>+        * rendering/RenderImage.h:
>+        Factor the missing-image outline out, and - if desired - paint it in
>+        cases where the image is still loading or otherwise pending, not just
>+        when the image fails to load.
>+
>+        * page/Settings.yaml:
>+        * testing/InternalSettings.cpp:
>+        (WebCore::InternalSettings::Backup::Backup):
>+        (WebCore::InternalSettings::Backup::restoreTo):
>+        (WebCore::InternalSettings::setIncompleteImageBorderEnabled):
>+        * testing/InternalSettings.h:
>+        * testing/InternalSettings.idl:
>+        Add and expose a setting to enable the feature.
>+
> 2018-06-12  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         REGRESSION(r228724): Occasional crash when executing ReplaceSelectionCommand at the end of the document
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ae68efe4c0b91b669f1451ecc5ad96a449f18319..dce4378d1d50bc4120f8a1fe7503bc9d611aefe7 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-06-13  Tim Horton  <timothy_horton@apple.com>
>+
>+        Make it possible to add a border around loading or failed-to-load images
>+        https://bugs.webkit.org/show_bug.cgi?id=186614
>+        <rdar://problem/39050152>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Shared/WebPreferences.yaml:
>+        * UIProcess/API/Cocoa/WKPreferences.mm:
>+        (-[WKPreferences _setIncompleteImageBorderEnabled:]):
>+        (-[WKPreferences _incompleteImageBorderEnabled]):
>+        * UIProcess/API/Cocoa/WKPreferencesPrivate.h:
>+        Plumb the setting to WebKit2.
>+
> 2018-06-12  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [WebKit on watchOS] Upstream watchOS source additions to OpenSource (Part 2)
>diff --git a/Source/WebCore/page/Settings.yaml b/Source/WebCore/page/Settings.yaml
>index dd9ed86fa1554cf7d76e49a1fe9667112ee6609a..c1c41b353212183895ee2d1032071741296f54fb 100644
>--- a/Source/WebCore/page/Settings.yaml
>+++ b/Source/WebCore/page/Settings.yaml
>@@ -750,3 +750,6 @@ crossOriginWindowPolicySupportEnabled:
> accessibilityEventsEnabled:
>   initial: true
>   conditional: ACCESSIBILITY_EVENTS
>+
>+incompleteImageBorderEnabled:
>+  initial: false
>diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
>index acd99a02a56d251123b5d9b7a27829a4a504abdb..cddc8c80d50f8ef402b6a6cc53136c5c8892edd8 100644
>--- a/Source/WebCore/rendering/RenderImage.cpp
>+++ b/Source/WebCore/rendering/RenderImage.cpp
>@@ -378,6 +378,25 @@ bool RenderImage::hasNonBitmapImage() const
>     return image && !is<BitmapImage>(image);
> }
> 
>+void RenderImage::paintIncompleteImageOutline(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const LayoutUnit& borderWidth)
>+{
>+    LayoutSize contentSize = this->contentSize();
>+    if (contentSize.width() <= 2 || contentSize.height() <= 2)
>+        return;
>+
>+    LayoutUnit leftBorder = borderLeft();
>+    LayoutUnit topBorder = borderTop();
>+    LayoutUnit leftPad = paddingLeft();
>+    LayoutUnit topPad = paddingTop();
>+
>+    // Draw an outline rect where the image should be.
>+    GraphicsContext& context = paintInfo.context();
>+    context.setStrokeStyle(SolidStroke);
>+    context.setStrokeColor(Color::lightGray);
>+    context.setFillColor(Color::transparent);
>+    context.drawRect(snapRectToDevicePixels(LayoutRect({ paintOffset.x() + leftBorder + leftPad, paintOffset.y() + topBorder + topPad }, contentSize), document().deviceScaleFactor()), borderWidth);
>+}
>+
> void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
> {
>     LayoutSize contentSize = this->contentSize();
>@@ -392,25 +411,20 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
>         if (paintInfo.phase == PaintPhaseForeground)
>             page().addRelevantUnpaintedObject(this, visualOverflowRect());
> 
>-        if (contentSize.width() > 2 && contentSize.height() > 2) {
>-            LayoutUnit borderWidth = LayoutUnit(1 / deviceScaleFactor);
>+        LayoutUnit missingImageBorderWidth = LayoutUnit(1 / deviceScaleFactor);
>+        paintIncompleteImageOutline(paintInfo, paintOffset, missingImageBorderWidth);
> 
>+        if (contentSize.width() > 2 && contentSize.height() > 2) {
>             LayoutUnit leftBorder = borderLeft();
>             LayoutUnit topBorder = borderTop();
>             LayoutUnit leftPad = paddingLeft();
>             LayoutUnit topPad = paddingTop();
> 
>-            // Draw an outline rect where the image should be.
>-            context.setStrokeStyle(SolidStroke);
>-            context.setStrokeColor(Color::lightGray);
>-            context.setFillColor(Color::transparent);
>-            context.drawRect(snapRectToDevicePixels(LayoutRect({ paintOffset.x() + leftBorder + leftPad, paintOffset.y() + topBorder + topPad }, contentSize), deviceScaleFactor), borderWidth);
>-
>             bool errorPictureDrawn = false;
>             LayoutSize imageOffset;
>             // When calculating the usable dimensions, exclude the pixels of
>             // the ouline rect so the error image/alt text doesn't draw on it.
>-            LayoutSize usableSize = contentSize - LayoutSize(2 * borderWidth, 2 * borderWidth);
>+            LayoutSize usableSize = contentSize - LayoutSize(2 * missingImageBorderWidth, 2 * missingImageBorderWidth);
> 
>             RefPtr<Image> image = imageResource().image();
> 
>@@ -427,7 +441,7 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
>                 LayoutUnit centerY = (usableSize.height() - imageSize.height()) / 2;
>                 if (centerY < 0)
>                     centerY = 0;
>-                imageOffset = LayoutSize(leftBorder + leftPad + centerX + borderWidth, topBorder + topPad + centerY + borderWidth);
>+                imageOffset = LayoutSize(leftBorder + leftPad + centerX + missingImageBorderWidth, topBorder + topPad + centerY + missingImageBorderWidth);
> 
>                 ImageOrientationDescription orientationDescription(shouldRespectImageOrientation());
> #if ENABLE(CSS_IMAGE_ORIENTATION)
>@@ -444,7 +458,7 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
>                 const FontMetrics& fontMetrics = font.fontMetrics();
>                 LayoutUnit ascent = fontMetrics.ascent();
>                 LayoutPoint altTextOffset = paintOffset;
>-                altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - borderWidth, topBorder + topPad + ascent + (paddingHeight / 2) - borderWidth);
>+                altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - missingImageBorderWidth, topBorder + topPad + ascent + (paddingHeight / 2) - missingImageBorderWidth);
> 
>                 // Only draw the alt text if it'll fit within the content box,
>                 // and only if it fits above the error image.
>@@ -463,8 +477,14 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
>     if (contentSize.isEmpty())
>         return;
> 
>+    LayoutUnit incompleteImageBorderWidth = LayoutUnit(2 / deviceScaleFactor);
>+    bool showBorderForIncompleteImage = settings().incompleteImageBorderEnabled();
>+
>     RefPtr<Image> img = imageResource().image(flooredIntSize(contentSize));
>     if (!img || img->isNull()) {
>+        if (showBorderForIncompleteImage)
>+            paintIncompleteImageOutline(paintInfo, paintOffset, incompleteImageBorderWidth);
>+
>         if (paintInfo.phase == PaintPhaseForeground)
>             page().addRelevantUnpaintedObject(this, visualOverflowRect());
>         return;
>@@ -480,6 +500,9 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
>         context.clip(contentBoxRect);
> 
>     ImageDrawResult result = paintIntoRect(paintInfo, snapRectToDevicePixels(replacedContentRect, deviceScaleFactor));
>+
>+    if (showBorderForIncompleteImage && (result != ImageDrawResult::DidDraw || (cachedImage() && cachedImage()->isLoading())))
>+        paintIncompleteImageOutline(paintInfo, paintOffset, incompleteImageBorderWidth);
>     
>     if (cachedImage() && paintInfo.phase == PaintPhaseForeground) {
>         // For now, count images as unpainted if they are still progressively loading. We may want 
>diff --git a/Source/WebCore/rendering/RenderImage.h b/Source/WebCore/rendering/RenderImage.h
>index 565431010ca2e98e190b094cdfc854df6b8931f3..d8bdc701b4b1779046177767c41c59795e19a629 100644
>--- a/Source/WebCore/rendering/RenderImage.h
>+++ b/Source/WebCore/rendering/RenderImage.h
>@@ -107,6 +107,7 @@ private:
>     bool isRenderImage() const final { return true; }
> 
>     void paintReplaced(PaintInfo&, const LayoutPoint&) override;
>+    void paintIncompleteImageOutline(PaintInfo&, const LayoutPoint&, const LayoutUnit&);
> 
>     bool computeBackgroundIsKnownToBeObscured(const LayoutPoint& paintOffset) final;
> 
>diff --git a/Source/WebCore/testing/InternalSettings.cpp b/Source/WebCore/testing/InternalSettings.cpp
>index 24e4aaebe3746e0204cb3e7f0affc71e27b54038..3f8fc860c799b023de8c8ec69b44d2da130d1b2d 100644
>--- a/Source/WebCore/testing/InternalSettings.cpp
>+++ b/Source/WebCore/testing/InternalSettings.cpp
>@@ -93,6 +93,7 @@ InternalSettings::Backup::Backup(Settings& settings)
>     , m_inlineMediaPlaybackRequiresPlaysInlineAttribute(settings.inlineMediaPlaybackRequiresPlaysInlineAttribute())
>     , m_deferredCSSParserEnabled(settings.deferredCSSParserEnabled())
>     , m_inputEventsEnabled(settings.inputEventsEnabled())
>+    , m_incompleteImageBorderEnabled(settings.incompleteImageBorderEnabled())
> #if ENABLE(ACCESSIBILITY_EVENTS)
>     , m_accessibilityEventsEnabled(settings.accessibilityEventsEnabled())
> #endif
>@@ -204,6 +205,7 @@ void InternalSettings::Backup::restoreTo(Settings& settings)
>     RenderTheme::singleton().setShouldMockBoldSystemFontForAccessibility(m_shouldMockBoldSystemFontForAccessibility);
>     FontCache::singleton().setShouldMockBoldSystemFontForAccessibility(m_shouldMockBoldSystemFontForAccessibility);
>     settings.setFrameFlattening(m_frameFlattening);
>+    settings.setIncompleteImageBorderEnabled(m_incompleteImageBorderEnabled);
> #if ENABLE(ACCESSIBILITY_EVENTS)
>     settings.setAccessibilityEventsEnabled(m_accessibilityEventsEnabled);
> #endif
>@@ -902,6 +904,14 @@ ExceptionOr<void> InternalSettings::setAccessibilityEventsEnabled(bool enabled)
>     return { };
> }
> 
>+ExceptionOr<void> InternalSettings::setIncompleteImageBorderEnabled(bool enabled)
>+{
>+    if (!m_page)
>+        return Exception { InvalidAccessError };
>+    settings().setIncompleteImageBorderEnabled(enabled);
>+    return { };
>+}
>+
> static InternalSettings::ForcedAccessibilityValue settingsToInternalSettingsValue(Settings::ForcedAccessibilityValue value)
> {
>     switch (value) {
>diff --git a/Source/WebCore/testing/InternalSettings.h b/Source/WebCore/testing/InternalSettings.h
>index 9a48f156c414442ab4fa9b4357e71fe3fb724b72..6707fe6aa192fb8745a2a7d83564480362728a37 100644
>--- a/Source/WebCore/testing/InternalSettings.h
>+++ b/Source/WebCore/testing/InternalSettings.h
>@@ -101,6 +101,7 @@ public:
>     ExceptionOr<void> setShouldManageAudioSessionCategory(bool);
>     ExceptionOr<void> setCustomPasteboardDataEnabled(bool);
>     ExceptionOr<void> setAccessibilityEventsEnabled(bool);
>+    ExceptionOr<void> setIncompleteImageBorderEnabled(bool);
> 
>     using FrameFlatteningValue = FrameFlattening;
>     ExceptionOr<void> setFrameFlattening(FrameFlatteningValue);
>@@ -192,6 +193,7 @@ private:
>         bool m_inlineMediaPlaybackRequiresPlaysInlineAttribute;
>         bool m_deferredCSSParserEnabled;
>         bool m_inputEventsEnabled;
>+        bool m_incompleteImageBorderEnabled;
> #if ENABLE(ACCESSIBILITY_EVENTS)
>         bool m_accessibilityEventsEnabled;
> #endif
>diff --git a/Source/WebCore/testing/InternalSettings.idl b/Source/WebCore/testing/InternalSettings.idl
>index ab6157f5cf8eb6617c317239d8a7a9e644c03846..d16986e8c28d9df332369ec3e1ad6d20197883f1 100644
>--- a/Source/WebCore/testing/InternalSettings.idl
>+++ b/Source/WebCore/testing/InternalSettings.idl
>@@ -85,6 +85,7 @@ enum FontLoadTimingOverride { "Block", "Swap", "Failure" };
>     [MayThrowException] void setAllowsInlineMediaPlaybackAfterFullscreen(boolean allows);
>     [MayThrowException] void setInlineMediaPlaybackRequiresPlaysInlineAttribute(boolean requires);
>     [MayThrowException] void setFrameFlattening(FrameFlatteningValue frameFlattening);
>+    [MayThrowException] void setIncompleteImageBorderEnabled(boolean enabled);
> 
>     // RuntimeEnabledFeatures.
>     void setIndexedDBWorkersEnabled(boolean enabled);
>diff --git a/Source/WebKit/Shared/WebPreferences.yaml b/Source/WebKit/Shared/WebPreferences.yaml
>index 54bffcb1e450573f5a3fdb9ae873299b9be0c800..96dd7bf1ee809a7a50a0f9fc017e0e3ae80a5bd0 100644
>--- a/Source/WebKit/Shared/WebPreferences.yaml
>+++ b/Source/WebKit/Shared/WebPreferences.yaml
>@@ -1297,3 +1297,7 @@ DisabledAdaptationsMetaTagEnabled:
> ColorFilterEnabled:
>   type: bool
>   defaultValue: false
>+
>+IncompleteImageBorderEnabled:
>+  type: bool
>+  defaultValue: false
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm b/Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm
>index 5fc0bc5a73af08d6188b1618cae1c79f5eddf42f..7967abd98d729c055f522bb651f8570c578230dc 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm
>@@ -1255,6 +1255,16 @@ - (BOOL)_shouldEnableTextAutosizingBoost
> #endif
> }
> 
>+- (void)_setIncompleteImageBorderEnabled:(BOOL)incompleteImageBorderEnabled
>+{
>+    _preferences->setIncompleteImageBorderEnabled(incompleteImageBorderEnabled);
>+}
>+
>+- (BOOL)_incompleteImageBorderEnabled
>+{
>+    return _preferences->incompleteImageBorderEnabled();
>+}
>+
> @end
> 
> #endif // WK_API_ENABLED
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h
>index c005fcbc27f340c1f7c9be23e9e1af1369fa2b08..6d64185d7a8c71f7944468667e72c1492f60b7eb 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h
>@@ -138,6 +138,8 @@ typedef NS_ENUM(NSInteger, _WKEditableLinkBehavior) {
> 
> @property (nonatomic, setter=_setStorageAccessPromptsEnabled:) BOOL _storageAccessPromptsEnabled WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
>+@property (nonatomic, setter=_setIncompleteImageBorderEnabled:) BOOL _incompleteImageBorderEnabled WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
>+
> #if !TARGET_OS_IPHONE
> @property (nonatomic, setter=_setWebGLEnabled:) BOOL _webGLEnabled WK_API_AVAILABLE(macosx(10.13.4));
> @property (nonatomic, setter=_setJavaEnabledForLocalFiles:) BOOL _javaEnabledForLocalFiles WK_API_AVAILABLE(macosx(10.13.4));
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6b69583db80fca61fd0dab7cedba05ec32beb5e4..f04ea5df6ebd37544f3c022d4813ded62058aa7f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-06-13  Tim Horton  <timothy_horton@apple.com>
>+
>+        Make it possible to add a border around loading or failed-to-load images
>+        https://bugs.webkit.org/show_bug.cgi?id=186614
>+        <rdar://problem/39050152>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/images/loading-image-border-expected.html: Added.
>+        * http/tests/images/loading-image-border.html: Added.
>+        * http/tests/images/loading-image-no-border-expected.html: Added.
>+        * http/tests/images/loading-image-no-border.html: Added.
>+        * platform/wk2/TestExpectations:
>+        Add a test ensuring that the setting works correctly.
>+        These and similar tests do not currently work in WebKitTestRunner, so they are skipped there.
>+
> 2018-06-12  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [WebKit on watchOS] Upstream watchOS source additions to OpenSource (Part 2)
>diff --git a/LayoutTests/http/tests/images/loading-image-border-expected.html b/LayoutTests/http/tests/images/loading-image-border-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..aa00d3f135b720b66b744bbbbd4f3e65cf6e0b53
>--- /dev/null
>+++ b/LayoutTests/http/tests/images/loading-image-border-expected.html
>@@ -0,0 +1,6 @@
>+<body>
>+    <img src="broken-image.png"  width="500px" height="500px">
>+
>+    
>+    <div style="width: 100px; height: 100px; position: absolute; top: 200px; left: 200px; background-color: white;">
>+</body>
>diff --git a/LayoutTests/http/tests/images/loading-image-border.html b/LayoutTests/http/tests/images/loading-image-border.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..3b10f9f294558c88b53929ce10d94daac8f7d2f4
>--- /dev/null
>+++ b/LayoutTests/http/tests/images/loading-image-border.html
>@@ -0,0 +1,14 @@
>+<body>
>+    <script>
>+        internals.settings.setIncompleteImageBorderEnabled(true);
>+
>+        window.onload = function () {
>+            img.src = "http://127.0.0.1:8000/resources/load-and-stall.php?name=../../../fast/images/resources/green-400x400.png&mimeType=image%2Fpng&stallAt=0&stallFor=100";
>+
>+            testRunner.forceImmediateCompletion();
>+        }
>+
>+        testRunner.waitUntilDone();
>+    </script>
>+    <img id="img" width="500px" height="500px">
>+</body>
>diff --git a/LayoutTests/http/tests/images/loading-image-no-border-expected.html b/LayoutTests/http/tests/images/loading-image-no-border-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
>diff --git a/LayoutTests/http/tests/images/loading-image-no-border.html b/LayoutTests/http/tests/images/loading-image-no-border.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..0bce7c6da1871accf56ed1227500609391a493f6
>--- /dev/null
>+++ b/LayoutTests/http/tests/images/loading-image-no-border.html
>@@ -0,0 +1,14 @@
>+<body>
>+    <script>
>+        internals.settings.setIncompleteImageBorderEnabled(false);
>+
>+        window.onload = function () {
>+            img.src = "http://127.0.0.1:8000/resources/load-and-stall.php?name=../../../fast/images/resources/green-400x400.png&mimeType=image%2Fpng&stallAt=0&stallFor=100";
>+
>+            testRunner.forceImmediateCompletion();
>+        }
>+
>+        testRunner.waitUntilDone();
>+    </script>
>+    <img id="img" width="500px" height="500px">
>+</body>
>diff --git a/LayoutTests/platform/wk2/TestExpectations b/LayoutTests/platform/wk2/TestExpectations
>index 895b2c1e199281e021d5dcd78442e7a8a7cd5783..356a8da3ed87b5e1d9bcf2be72ec5998057ea0b2 100644
>--- a/LayoutTests/platform/wk2/TestExpectations
>+++ b/LayoutTests/platform/wk2/TestExpectations
>@@ -199,6 +199,11 @@ http/tests/download/default-encoding.html
> http/tests/download/inherited-encoding-form-submission-result.html
> http/tests/download/inherited-encoding.html
> 
>+# Incomplete-image tests fail in WebKitTestRunner
>+# https://bugs.webkit.org/show_bug.cgi?id=186613
>+http/tests/images/loading-image-border.html
>+http/tests/images/loading-image-no-border.html
>+
> ### END OF (1) Classified failures with bug reports
> ########################################
>

Actions: View | Formatted Diff | Diff

Attachments on bug 186614: 342717 | 342718 | 342719 | 342771 | 343218 | 343222 | 343223 | 343237 | 343361 | 343392