[Webkit-unassigned] [Bug 153361] AX: new lines in content editable elements don't notify accessibility

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 25 14:05:48 PST 2016


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

--- Comment #9 from Doug Russell <d_russell at apple.com> ---
(In reply to comment #8)
> Comment on attachment 272234 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272234&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:121
> > +    if (AXObjectCache::accessibilityEnabled() && selection.isRange())
> 
> if ax is not enabled it seems like we should not do anything

Meaning we shouldn't initialize m_replacedText?

> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:124
> > +        m_replacedText = String();
> 
> this seems unnecessary. i think this is what m_replacedText will be
> initialized to regardless

I'll have to check.

> 
> > Source/WebCore/editing/CompositeEditCommand.cpp:637
> > +    default:
> 
> feel like we should ASSERT_NOT_REACHED for default: and expect someone will
> pass in Cut or Delete

Will do.

> 
> > Source/WebCore/editing/DictationCommand.cpp:113
> > +    notifyAccessibilityForTextChange(AXTextEditTypeDictation, m_textToInsert)
> 
> check Internals.cpp for whether dictation has some stubs to direct data
> through

Will do.

> 
> > Source/WebCore/editing/Editor.cpp:543
> > +    applyCommand(cmd);
> 
> this change seems unneeded

I need it for cmd->replacedText()

> 
> > Source/WebCore/editing/Editor.cpp:557
> > +    if (AXObjectCache::accessibilityEnabled() && editingAction == EditActionPaste)
> 
> seems like we should cleanup memory when not being used

I'm inclined to reclaim the cmd->replacedText() memory after this if regardless since it's not used again (although it might be in the incomplete undo logic.

> 
> > Source/WebCore/editing/Editor.cpp:1283
> > +    cache->postTextStateChangeNotification(node, AXTextEditTypeCut, text, start);
> 
> do we need to check for node == nil here? can postTextState changes handle
> the nil node case instead?

In the nil node case postTextStateChangeNotification ends up using the web area for the notification (on OS X) so I could drop that check.

> 
> > Source/WebCore/editing/InsertTextCommand.cpp:143
> > +        // FIXME: Need to capture the deleted selection and post replacement notification after insertion
> 
> any fixme: should probably have a webkit bug tied to it

That's out of date. I'll remove it.

> 
> > LayoutTests/accessibility/mac/value-change-userinfo.html:195
> > +//             execExtendSelectionRightByCharacterCommand();
> 
> should this be deleted?

Yes.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160225/9a3883dc/attachment.html>


More information about the webkit-unassigned mailing list