<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AX:Implement next/previous text marker functions using TextIterator"
href="https://bugs.webkit.org/show_bug.cgi?id=152728#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AX:Implement next/previous text marker functions using TextIterator"
href="https://bugs.webkit.org/show_bug.cgi?id=152728">bug 152728</a>
from <span class="vcard"><a class="email" href="mailto:cfleizach@apple.com" title="chris fleizach <cfleizach@apple.com>"> <span class="fn">chris fleizach</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=268345&action=diff" name="attach_268345" title="patch">attachment 268345</a> <a href="attachment.cgi?id=268345&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=268345&action=review">https://bugs.webkit.org/attachment.cgi?id=268345&action=review</a>
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1445
> + // Here when we don't have any character to move and we are going backwards, we traverse to previous node.</span >
traverse to "the" previous
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1462
> + int suboffset = iterator.range()->startOffset();</span >
suboffset -> subOffset
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1470
> + if (currentLength == 1 && (iterator.text()[0] == ' ' || iterator.text()[0] == '\n' || iterator.text()[0] == '\t'))</span >
did this line come from existing code? My guess is that there's some whitespace detection mechanism that will account for other weird things that you don't have here like \r
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1484
> + offsetInCharacter = offset - (offsetSoFar - currentLength);</span >
bad spacing after =
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1513
> +void AXObjectCache::startOrEndTextMarkerDataforRange(TextMarkerData& textMarkerData, RefPtr<Range> range, bool isStart)</span >
Datafor -> DataFor
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1528
> + if (is<HTMLInputElement>(*domNode) && downcast<HTMLInputElement>(*domNode).isPasswordField())</span >
we should add a layout test variation for the password field case
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1531
> + AXObjectCache* cache = domNode->document().axObjectCache();</span >
do we think that this domNode will have a different AXObjectCache then this axObjectCache that we're in right now?
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1583
> + domNode = charOffset.node;</span >
this bit of code is pretty identical to the method above this one. Can we make a helper routine to consolidate the code?
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:1669
> + AXObjectCache* cache = domNode->document().axObjectCache();</span >
seems like we should be using this AXObjectCache instead of asking for the cache through the document
<span class="quote">> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:503
> + if (!textMarker)</span >
we should check for !cache case too just to be safe
<span class="quote">> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3866
> + return [self textMarkerForNode:*textMarkerData.node Offset:textMarkerData.characterOffset + 1];</span >
Offset should be lowercase
<span class="quote">> LayoutTests/accessibility/mac/previous-next-text-marker.html:96
> + shouldBeTrue("text3.accessibilityElementForTextMarker(currentMarker).isEqual(text3)");</span >
let's add a check here that does
1. iterate into the user-select: none text
2. we get two markers that represent positions within user-select:none (like from "s" -> "l" or something)
3. check the string that we get back from this range and verify text is correct
4. then let's iterate backwards from that 2nd node and get another range where we verify the string
thanks</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>