Changeset 275076 in webkit

Timestamp:

Mar 25, 2021, 8:27:16 PM (5 years ago)

Author:

Wenson Hsieh

Message:

Don't add -webkit-user-select: none; on image elements with draggable=true
​https://bugs.webkit.org/show_bug.cgi?id=223774
<rdar://problem/75860124>

Reviewed by Tim Horton.

Source/WebCore:

Avoid adding this presentational CSS style property for image elements marked with draggable=true. Since
image elements are already draggable by default and mouse drags over image elements do not trigger text
selection, it's not necessary for the user agent to add this style property. See below for more details.

Test: fast/images/image-extraction/mac/select-word-in-draggable-image-overlay.html

• html/HTMLElement.cpp:

(WebCore::HTMLElement::collectStyleForPresentationAttribute):

If the element is already draggable (barring HTML attributes), don't additionally disable text selection inside
the element when we additionally have draggable=true set on the element.

(WebCore::HTMLElement::draggable const):

Refactor this to consult isDraggableIgnoringAttributes() when determining whether to check if the draggable
attribute value is not "false" vs. equal to "true". In the case where the element is already draggable, we
only return false here if draggable=false is explicitly set.

• html/HTMLElement.h:

(WebCore::HTMLElement::isDraggableIgnoringAttributes const):

• html/HTMLImageElement.cpp:

(WebCore::HTMLImageElement::draggable const): Deleted.

• html/HTMLImageElement.h:

Override isDraggableIgnoringAttributes and return true.

LayoutTests:

• fast/images/image-extraction/mac/select-word-in-draggable-image-overlay-expected-mismatch.html: Added.
• fast/images/image-extraction/mac/select-word-in-draggable-image-overlay.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/images/image-extraction/mac/select-word-in-draggable-image-overlay-expected-mismatch.html (added)
• LayoutTests/fast/images/image-extraction/mac/select-word-in-draggable-image-overlay.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLElement.h (modified) (1 diff)
• Source/WebCore/html/HTMLImageElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLImageElement.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-03-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Don't add `-webkit-user-select: none;` on image elements with `draggable=true`
+        https://bugs.webkit.org/show_bug.cgi?id=223774
+        <rdar://problem/75860124>
+
+        Reviewed by Tim Horton.
+
+        * fast/images/image-extraction/mac/select-word-in-draggable-image-overlay-expected-mismatch.html: Added.
+        * fast/images/image-extraction/mac/select-word-in-draggable-image-overlay.html: Added.
+
 2021-03-25  Kyle Piddington  <kpiddington@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Don't add `-webkit-user-select: none;` on image elements with `draggable=true`
+        https://bugs.webkit.org/show_bug.cgi?id=223774
+        <rdar://problem/75860124>
+
+        Reviewed by Tim Horton.
+
+        Avoid adding this presentational CSS style property for image elements marked with `draggable=true`. Since
+        image elements are already draggable by default and mouse drags over image elements do not trigger text
+        selection, it's not necessary for the user agent to add this style property. See below for more details.
+
+        Test: fast/images/image-extraction/mac/select-word-in-draggable-image-overlay.html
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::collectStyleForPresentationAttribute):
+
+        If the element is already draggable (barring HTML attributes), don't additionally disable text selection inside
+        the element when we additionally have `draggable=true` set on the element.
+
+        (WebCore::HTMLElement::draggable const):
+
+        Refactor this to consult `isDraggableIgnoringAttributes()` when determining whether to check if the `draggable`
+        attribute value is not `"false"` vs. equal to `"true"`. In the case where the element is already draggable, we
+        only return `false` here if `draggable=false` is explicitly set.
+
+        * html/HTMLElement.h:
+        (WebCore::HTMLElement::isDraggableIgnoringAttributes const):
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::draggable const): Deleted.
+        * html/HTMLImageElement.h:
+
+        Override `isDraggableIgnoringAttributes` and return `true`.
+
 2021-03-25  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/html/HTMLElement.cpp

@@ -211 +211 @@
         if (equalLettersIgnoringASCIICase(value, "true")) {
             addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserDrag, CSSValueElement);
-            addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserSelect, CSSValueNone);
+            if (!isDraggableIgnoringAttributes())
+                addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserSelect, CSSValueNone);
         } else if (equalLettersIgnoringASCIICase(value, "false"))
             addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserDrag, CSSValueNone);
@@ -710 +711 @@
 bool HTMLElement::draggable() const
 {
-    return equalLettersIgnoringASCIICase(attributeWithoutSynchronization(draggableAttr), "true");
+    auto& value = attributeWithoutSynchronization(draggableAttr);
+    if (isDraggableIgnoringAttributes())
+        return !equalLettersIgnoringASCIICase(value, "false");
+
+    return equalLettersIgnoringASCIICase(value, "true");
 }
 

trunk/Source/WebCore/html/HTMLElement.h

@@ -64 +64 @@
     virtual bool draggable() const;
     WEBCORE_EXPORT void setDraggable(bool);
+    virtual bool isDraggableIgnoringAttributes() const { return false; }
 
     WEBCORE_EXPORT bool spellcheck() const;

trunk/Source/WebCore/html/HTMLImageElement.cpp

@@ -559 +559 @@
 }
 
-bool HTMLImageElement::draggable() const
-{
-    // Image elements are draggable by default.
-    return !equalLettersIgnoringASCIICase(attributeWithoutSynchronization(draggableAttr), "false");
-}
-
 void HTMLImageElement::setHeight(unsigned value)
 {

trunk/Source/WebCore/html/HTMLImageElement.h

@@ -162 +162 @@
     String completeURLsInAttributeValue(const URL& base, const Attribute&) const override;
 
-    bool draggable() const override;
+    bool isDraggableIgnoringAttributes() const final { return true; }
 
     void addSubresourceAttributeURLs(ListHashSet<URL>&) const override;