Bug 196670 – [iOS] Moving backwards by word granularity does not work if the previous line was inside another block element

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

With the following markup, iOS WebKit fails to move backwards from the beginning of the second line by word granularity:

<div>first</div>
<div>second</div>

<rdar://problem/46880663>

[Ryosuke Niwa]

Created attachment 366875 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 366876 [details]
Fixes the bug

[EWS Watchlist]

Comment on attachment 366876 [details]
Fixes the bug

Attachment 366876 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11788918

New failing tests:
editing/text-iterator/backwards-text-iterator-basic.html
fast/dom/Range/range-expand.html
accessibility/mac/text-marker-sentence-nav.html

[EWS Watchlist]

Created attachment 366878 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366876 [details]
Fixes the bug

Attachment 366876 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11788930

New failing tests:
editing/text-iterator/backwards-text-iterator-basic.html
accessibility/mac/text-marker-sentence-nav.html
fast/dom/Range/range-expand.html

[EWS Watchlist]

Created attachment 366879 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366876 [details]
Fixes the bug

Attachment 366876 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11789034

New failing tests:
editing/text-iterator/backwards-text-iterator-basic.html
fast/dom/Range/range-expand.html
accessibility/mac/text-marker-sentence-nav.html

[EWS Watchlist]

Created attachment 366880 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366876 [details]
Fixes the bug

Attachment 366876 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11789045

New failing tests:
editing/text-iterator/backwards-text-iterator-basic.html

[EWS Watchlist]

Created attachment 366881 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366876 [details]
Fixes the bug

Attachment 366876 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11789177

New failing tests:
fast/dom/Range/range-expand.html

[EWS Watchlist]

Created attachment 366882 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Ryosuke Niwa]

Created attachment 367030 [details]
WIP

The previous patch revealed a bug in startOfSentence. When I fix that, AXObjectCache::previousBoundary in turn reveals another bug about the way we're failing to take care of suffixLength affecting the text index within a text node.

So I'd have to fix 3-4 unrelated bugs that used to cancel one another before this patch at once :(

[Ryosuke Niwa]

Created attachment 367084 [details]
Fixes the bug

[Alexey Proskuryakov]

Comment on attachment 367084 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=367084&action=review

> Source/WebCore/editing/TextIterator.h:53
> +// For testing.
> +WEBCORE_EXPORT String plainTextUsingBackwardsTextIterator(const Range&);

I didn't look at the rest of the patch, but this caught my eye. Why not add "ForTesting" to the function name?

[Ryosuke Niwa]

(In reply to Alexey Proskuryakov from comment #15)
> Comment on attachment 367084 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367084&action=review
> 
> > Source/WebCore/editing/TextIterator.h:53
> > +// For testing.
> > +WEBCORE_EXPORT String plainTextUsingBackwardsTextIterator(const Range&);
> 
> I didn't look at the rest of the patch, but this caught my eye. Why not add
> "ForTesting" to the function name?

Sure, I can add that suffix.

[Ryosuke Niwa]

Created attachment 367203 [details]
Added ForTesting suffix to plainTextUsingBackwardsTextIterator

[Wenson Hsieh]

Comment on attachment 367203 [details]
Added ForTesting suffix to plainTextUsingBackwardsTextIterator

View in context: https://bugs.webkit.org/attachment.cgi?id=367203&action=review

> Source/WebCore/ChangeLog:17
> +        make SimplifiedBackwardsTextIterator directly testable in layout tests.

Nit - s/make/to make/

[Ryosuke Niwa]

Committed r244200: <https://trac.webkit.org/changeset/244200>