<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - AX: Expose caret browsing preference to accessibility API"
   href="https://bugs.webkit.org/show_bug.cgi?id=141862#c4">Comment # 4</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - AX: Expose caret browsing preference to accessibility API"
   href="https://bugs.webkit.org/show_bug.cgi?id=141862">bug 141862</a>
              from <span class="vcard"><a class="email" href="mailto:d_russell@apple.com" title="Doug Russell <d_russell@apple.com>"> <span class="fn">Doug Russell</span></a>
</span></b>
        <pre>(In reply to <a href="show_bug.cgi?id=141862#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=247025&action=diff" name="attach_247025" title="Patch">attachment 247025</a> <a href="attachment.cgi?id=247025&action=edit" title="Patch">[details]</a></span>
> Patch

> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=247025&action=review">https://bugs.webkit.org/attachment.cgi?id=247025&action=review</a>

> > Source/WebCore/accessibility/AccessibilityObject.h:978
> > +    virtual bool caretBrowsingEnabled() const { return false; }

> I would just put these implementations into AccessibilityObject and remove
> from AccessibilityRenderObject. It doesn't seem like there's a benefit to
> making those virtual</span >

The methods are virtual because AccessibilityObject doesn't know about the Frame that caretBrowsingEnabled is ultimately set on.

FrameView* AccessibilityRenderObject::documentFrameView() const

<span class="quote">> 
> > Tools/DumpRenderTree/AccessibilityUIElement.cpp:1404
> > +    return 0;

> return nullptr</span >

Wil do

<span class="quote">> 
> > LayoutTests/platform/mac/accessibility/caret-browsing-attribute.html:13
> > +    var webArea = accessibilityController.focusedElement;

> the body focus() idiom i think might have some issues on GTK. I usually
> prefer accessibilityController.rootElement.childAtIndex(0) I think. Or maybe
> the rootElement is the web area</span >

Wil do

<span class="quote">> 
> > LayoutTests/platform/mac/accessibility/caret-browsing-tab-selection.html:22
> > +            shouldBe("selectedElement(webArea)", "");

> can you (in general) put some new lines between tests units, to make it a
> bit easier parse</span >

Wil do

<span class="quote">> 
> > LayoutTests/platform/mac/accessibility/resources/accessibility-helper.js:11
> > +function selectedElement(webArea) {

> can you name this a little better so its clear its related to text marker
> selection</span >

Does

function elementAtStartMarkerOfSelectedTextMarkerRange(webArea)

work as far as clarity?</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>