Attachment 371006 Details for Bug 194754 – Patch

[patch] Patch

bug-194754-20190530180238.patch (text/plain), 8.46 KB, created by gr3g on 2019-05-30 18:02:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: gr3g

Created: 2019-05-30 18:02:38 PDT

Size: 8.46 KB

patch

obsolete

>Subversion Revision: 245620
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 65957a86b220418a9a15a08e785319b0ec1b6679..a557f0693b2d3f44504bf9c2c8e611ff456e72f3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-29  Greg Doolittle  <gr3g@apple.com>
>+
>+        Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
>+        https://bugs.webkit.org/show_bug.cgi?id=194754
>+        <rdar://problem/48144534>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: accessibility/img-alt-attribute-empty-string.html
>+               accessibility/img-alt-attribute-no-value.html
>+
>+        * accessibility/AccessibilityObject.cpp:
>+        (WebCore::AccessibilityObject::computedRoleString const):
>+
> 2019-05-21  Jer Noble  <jer.noble@apple.com>
> 
>         Media controls don't show in WK2 video fullscreen sometimes
>diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
>index f702fde3ce8e406000166356bfc98debac8040be..b23645a199ab742540413d5c628086a08ba85b37 100644
>--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp
>@@ -2517,7 +2517,19 @@ String AccessibilityObject::computedRoleString() const
> 
>     if (role == AccessibilityRole::LandmarkDocRegion)
>         return reverseAriaRoleMap().get(static_cast<int>(AccessibilityRole::LandmarkRegion));
>-
>+    if (role == AccessibilityRole::Image) {
>+        // if an image element has an alt attribute AND the attribute is
>+        // unassigned or assigned to empty string, role should be updated
>+        // to presentational
>+        if (hasAttribute(altAttr)) {
>+            // if alt is preseent, check if empty string or unassigned
>+            const AtomicString& alt = getAttribute(altAttr);
>+            if (alt.isEmpty())
>+                return reverseAriaRoleMap().get(static_cast<int>(AccessibilityRole::Presentational));
>+        }
>+        // if no alt present, or alt is not empty, keep the role as image
>+        return reverseAriaRoleMap().get(static_cast<int>(AccessibilityRole::Image));
>+    }
>     return reverseAriaRoleMap().get(static_cast<int>(role));
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 3360915e7923a934366b3e9ff38a235a0d7841f8..22b8aa009c0e7bc4741a3e016900a29e36cb9c99 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-05-29  Greg Doolittle  <gr3g@apple.com>
>+
>+        Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
>+        https://bugs.webkit.org/show_bug.cgi?id=194754
>+        <rdar://problem/48144534>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/img-alt-attribute-empty-string-expected.txt: Added.
>+        * accessibility/img-alt-attribute-empty-string.html: Added.
>+        * accessibility/img-alt-attribute-no-value-expected.txt: Added.
>+        * accessibility/img-alt-attribute-no-value.html: Added.
>+
> 2019-05-22  Per Arne Vollan  <pvollan@apple.com>
> 
>         Layout Test http/tests/security/showModalDialog-sync-cross-origin-page-load2.html is failing
>diff --git a/LayoutTests/accessibility/img-alt-attribute-empty-string-expected.txt b/LayoutTests/accessibility/img-alt-attribute-empty-string-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..29d007461d1d55198472d854f9094d16d560326f
>--- /dev/null
>+++ b/LayoutTests/accessibility/img-alt-attribute-empty-string-expected.txt
>@@ -0,0 +1,15 @@
>+
>+
>+
>+This tests that img elements with alt attribute of empty string are ignored.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS imagesGroup.childrenCount is 2
>+PASS platformValueForW3CName(imagesGroup.childAtIndex(0)) is "cake"
>+PASS platformValueForW3CName(imagesGroup.childAtIndex(1)) is "more cake"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/accessibility/img-alt-attribute-empty-string.html b/LayoutTests/accessibility/img-alt-attribute-empty-string.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e6915734e8cea46f4c7e6fda9356b920e6481cf8
>--- /dev/null
>+++ b/LayoutTests/accessibility/img-alt-attribute-empty-string.html
>@@ -0,0 +1,38 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../resources/js-test-pre.js"></script>
>+<script src="../resources/accessibility-helper.js"></script>
>+</head>
>+<body id="body">
>+
>+<div tabindex="0" role="group" id="images">
>+    <img alt="cake" src="resources/cake.png"><br>
>+    <img alt="" src="resources/cake.png" class="empty-string"><br>
>+    <img alt="more cake" src="resources/cake.png"><br>
>+</div>
>+
>+<p id="description"></p>
>+<div id="console"></div>
>+
>+<script>
>+
>+    description("This tests that img elements with alt attribute of empty string are ignored.");
>+
>+    if (window.accessibilityController) {
>+
>+        document.getElementById("images").focus();
>+        var imagesGroup = accessibilityController.focusedElement;
>+        // <img alt="" /> should be ignored, so the count should be 2:
>+        shouldBe("imagesGroup.childrenCount", "2"); 
>+        // make sure alt text is being read before and after
>+        shouldBeEqualToString("platformValueForW3CName(imagesGroup.childAtIndex(0))", "cake");
>+        shouldBeEqualToString("platformValueForW3CName(imagesGroup.childAtIndex(1))", "more cake");
>+    }
>+
>+</script>
>+
>+<script src="../resources/js-test-post.js"></script>
>+</body>
>+</html>
>+
>diff --git a/LayoutTests/accessibility/img-alt-attribute-no-value-expected.txt b/LayoutTests/accessibility/img-alt-attribute-no-value-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..392d42a14486c97077a3a3dc410c64219bc484d1
>--- /dev/null
>+++ b/LayoutTests/accessibility/img-alt-attribute-no-value-expected.txt
>@@ -0,0 +1,16 @@
>+
>+
>+
>+
>+This tests that img elements with an alt attribute and no assigned value are ignored.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS imagesGroup.childrenCount is 2
>+PASS platformValueForW3CName(imagesGroup.childAtIndex(0)) is "cake0"
>+PASS platformValueForW3CName(imagesGroup.childAtIndex(1)) is "cake3"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/accessibility/img-alt-attribute-no-value.html b/LayoutTests/accessibility/img-alt-attribute-no-value.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..73bd741a83d81af07b96a6247354731d65340158
>--- /dev/null
>+++ b/LayoutTests/accessibility/img-alt-attribute-no-value.html
>@@ -0,0 +1,44 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../resources/js-test-pre.js"></script>
>+<script src="../resources/accessibility-helper.js"></script>
>+</head>
>+<body id="body">
>+
>+<div tabindex="0" role="group" id="images">
>+    <img alt="cake0" src="resources/cake.png"><br>
>+    <img alt class="unassigned-alt cake1" src="resources/cake.png"><br>
>+    <img class="no-alt cake2" src="resources/cake.png"><br>
>+    <img alt="cake3" src="resources/cake.png"><br>
>+</div>
>+
>+<p id="description"></p>
>+<div id="console"></div>
>+
>+<script>
>+
>+description("This tests that img elements with an alt attribute and no assigned value are ignored.");
>+if (window.accessibilityController) {
>+    document.getElementById("images").focus();
>+    var imagesGroup = accessibilityController.focusedElement;
>+    // there are 4 images, but one has an alt attribute that is unassigned so it should be
>+    // ignored. there is a second image without any alt attribute at all. this image should
>+    // not be ignored. accessibilityController fails to recognize it. this has been
>+    // documented in <rdar://problem/51283943>.
>+    // as a result the image count is 2, but it should be 3.
>+    shouldBe("imagesGroup.childrenCount", "2");
>+    // make sure alt text is being read before and after
>+    shouldBeEqualToString("platformValueForW3CName(imagesGroup.childAtIndex(0))", "cake0");
>+    // accessiblityController should see the <img> element w/o alt attribute as an image,
>+    // but it erroneously classifies it as presentation.. so it won't find the image and use the filename as alt text
>+    // shouldBeEqualToString("platformValueForW3CName(imagesGroup.childAtIndex(2))", "cake.png");
>+    shouldBeEqualToString("platformValueForW3CName(imagesGroup.childAtIndex(1))", "cake3");
>+}
>+
>+</script>
>+
>+<script src="../resources/js-test-post.js"></script>
>+</body>
>+</html>
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 194754: 371006 | 371801 | 371834 | 372088 | 372090 | 372091 | 372098 | 372100 | 372103 | 372631 | 372729