Bug 45116 - AX: accessibility not returning strings when visibility is hidden
Summary: AX: accessibility not returning strings when visibility is hidden
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: chris fleizach
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-02 10:15 PDT by chris fleizach
Modified: 2010-09-10 11:35 PDT (History)
3 users (show)

See Also:


Attachments
Patch (12.68 KB, patch)
2010-09-02 11:19 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (13.18 KB, patch)
2010-09-02 11:30 PDT, chris fleizach
ddkilzer: review+
ddkilzer: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2010-09-02 10:15:39 PDT
when an item has a height = 0, or something is not visible, accessibility is not getting the textUnderELement(). That's because
TextIterators explicitly removing hidden elements. That's not appropriate for accessibility always
Comment 1 chris fleizach 2010-09-02 11:19:32 PDT
Created attachment 66389 [details]
Patch
Comment 2 chris fleizach 2010-09-02 11:30:25 PDT
Created attachment 66390 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2010-09-10 07:48:52 PDT
Comment on attachment 66390 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66390&action=prettypatch

> WebCore/WebCore.xcodeproj/project.pbxproj:20494
> +			developmentRegion = English;
This change should not be committed.

> WebCore/editing/TextIterator.cpp:2231
> +    int behavior = (isDisplayString) ? TextIteratorDefaultBehavior: TextIteratorEmitsTextsWithoutTranscoding;
The 'behavior' variable should be of type TextIteratorBehavior here.

No need for parenthesis around isDisplayString here.

> WebCore/editing/TextIterator.cpp:2234
> +    for (TextIterator it(r, static_cast<TextIteratorBehavior>(behavior)); !it.atEnd(); it.advance()) {
Then there is no need to cast 'behavior' to TextIteratorBehavior here.

> WebCore/editing/TextIterator.h:51
> +String plainText(const Range*, bool ignoreStyleVisibility = false);
It would be nice if this could be an enum (maybe use TextIteratorBehavior?) here instead of just a boolean to make it easier to read the code at call sites without referring to the argument list.  This change is not necessary to land this patch, though.

> WebCore/editing/TextIterator.h:52
> +UChar* plainTextToMallocAllocatedBuffer(const Range*, unsigned& bufferLength, bool isDisplayString, bool ignoreStyleVisibility = false);
Ditto.

> WebCore/editing/TextIterator.h:80
> +    TextIteratorIgnoreStyleVisibility = 1 << 4
This should be named "TextIteratorIgnoresStyleVisibility" to match the verb tense of the other enums.

> WebCore/editing/TextIterator.h:178
> +    bool m_ignoreStyleVisibility;
This should be named "m_ignoresStyleVisibility" to match the verb tense of other ivars.

> LayoutTests/platform/mac/accessibility/clipped-text-under-element.html:20
> +  <h1><a id="hidden-link" tabindex=0 href="http://domain.com/path/i-am-the-URL?id=1234">I am the link text.</a></h1>
Change the description to:  "I am the hidden link text."

> LayoutTests/platform/mac/accessibility/clipped-text-under-element.html:26
> +  <h1><a id="visible-link" tabindex=0 href="http://domain.com/path/i-am-the-URL?id=1234">I am the link text.</a></h1>
Change the description to:  "I am the visible link text."

> LayoutTests/platform/mac/accessibility/clipped-text-under-element.html:34
> +    description("This tests that even if an element is not visible, textUnderElement() will still give the text");
Nit: Add period at end of the description sentence.

> LayoutTests/platform/mac/accessibility/clipped-text-under-element.html:38
> +        var link1 = accessibilityController.focusedElement;
Use "hiddenLink" instead of "link1" here.

> LayoutTests/platform/mac/accessibility/clipped-text-under-element.html:42
> +        var link2 = accessibilityController.focusedElement;
Use "visibleLink" instead of "link2" here.
Comment 4 chris fleizach 2010-09-10 11:35:58 PDT
http://trac.webkit.org/changeset/67219