Bug 41817

Summary: AX: Misspellings not shown in AXAttributedStringForTextMarkerRange when selection is on word
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, ddkilzer, enrica, gustavo, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
patch to test against other bots ddkilzer: review+

Description chris fleizach 2010-07-07 17:31:47 PDT
the misspelling attribute should continue to be returned even when the selection is on that piece of misspelled.   The red underline for misspelled text temporarily disappear when the insertion point is within the misspelled text.  That does not mean the misspelled attribute is not available.  The misspelled attribute should still be returned in this case.
Comment 1 chris fleizach 2010-07-08 09:02:26 PDT
Right now when asking for an attributed string for a range, it returns appropriate markers like misspelled, color, font, etc.

the problem is that when the cursor is on a misspelled word, the misspelled underline disappears visually and AX no longer says that this word is misspelled

The fix I have here is to not rely on the document's cached markings, because they don't account for this case, and instead just check the spelling of the range.
Comment 2 chris fleizach 2010-07-08 12:03:17 PDT
I have two layout tests. One to handle the case where misspelled attribute was not returned when selection was on the word

and 2) to generally test the misspelled attributed string stuff since there was no layout test before and because i re-wrote the method
Comment 3 chris fleizach 2010-07-08 12:05:46 PDT
Created attachment 60924 [details]
Patch
Comment 4 WebKit Review Bot 2010-07-08 12:58:36 PDT
Attachment 60924 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3453042
Comment 5 chris fleizach 2010-07-08 14:51:22 PDT
Created attachment 60965 [details]
Patch
Comment 6 chris fleizach 2010-07-24 19:51:21 PDT
i added the DRT changes and the other layout test in a separate bug. this bug will now just address the actual issue
Comment 7 chris fleizach 2010-07-24 20:31:08 PDT
Created attachment 62519 [details]
patch to test against other bots
Comment 8 David Kilzer (:ddkilzer) 2010-07-24 22:21:43 PDT
Comment on attachment 62519 [details]
patch to test against other bots

WebCore/ChangeLog:10
 +          circumstances (like when selection has not moved across words, of if the cursor is in the middle

Typo: "of if the cursor" should be "or if the cursor".

WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:396
 +      // Check the spelling directly since document->markersForNode() does not store the misspelled marking when the cursor is in a word.

Should document->markersforNode() store the misspelled marking?  I'm not an editing expert, but that fix may be simpler if it makes sense (and doesn't cause any performance issues).  I CCed Enrica on this bug in case she has input.

Other than that the patch looks fine, but I think someone with more text editing knowledge should look over the patch before I r+ it.
Comment 9 chris fleizach 2010-07-24 23:23:32 PDT
> Should document->markersforNode() store the misspelled marking?  I'm not an editing expert, but that fix may be simpler if it makes sense (and doesn't cause any performance issues).  I CCed Enrica on this bug in case she has input.
> 

From what I discovered that method mirrors what is displayed and when the visuals update it relies on the cache inside there. It also doesn't get updated until selection is moved within the range


> Other than that the patch looks fine, but I think someone with more text editing knowledge should look over the patch before I r+ it.
Comment 10 David Kilzer (:ddkilzer) 2010-07-27 11:56:55 PDT
Comment on attachment 62519 [details]
patch to test against other bots

I looked over the patch again and this seems fine.  r=me
Comment 11 chris fleizach 2010-07-27 12:39:05 PDT
http://trac.webkit.org/changeset/64149