[Webkit-unassigned] [Bug 119829] IAccessibleText/2 implementation for AppleWindows port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119829





--- Comment #21 from chris fleizach <cfleizach at apple.com>  2013-08-23 16:22:18 PST ---
(In reply to comment #20)
> (In reply to comment #19)
> > (From update of attachment 209523 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=209523&action=review
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:130
> > > +    if (m_object->renderer()->document()->frame()->selection().isNone())
> > 
> > You should use m_object->document() and ensure existence of document()
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:235
> > > +    m_object->renderer()->document()->frame()->selection().clear();
> > 
> > ditto about document()
> 
> Is checking the existence of m_object->renderer()->document() not the same thing?
> I do that in the initialCheck method.

I think it's cleaner if we don't have to ask for the renderer() object. I think the platform implementations should try hard to work mainly through the WebCore AccessibilityObject interface rather than diving into objects it holds on to.

There's also a chance that renderer() disappears or doesn't exist for the object because maybe in a <canvas>

> 
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:548
> > > +    ASSERT(m_object->renderer()->document()->frame());
> > 
> > why do you need to assert frame() here?
> 
> Oops I don't.

(In reply to comment #20)
> (In reply to comment #19)
> > (From update of attachment 209523 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=209523&action=review
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:130
> > > +    if (m_object->renderer()->document()->frame()->selection().isNone())
> > 
> > You should use m_object->document() and ensure existence of document()
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:235
> > > +    m_object->renderer()->document()->frame()->selection().clear();
> > 
> > ditto about document()
> 
> Is checking the existence of m_object->renderer()->document() not the same thing?
> I do that in the initialCheck method.
> 
> > 
> > > Source/WebKit/win/AccessibleTextImpl.cpp:548
> > > +    ASSERT(m_object->renderer()->document()->frame());
> > 
> > why do you need to assert frame() here?
> 
> Oops I don't.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.