Bug 186424

Summary: REGRESSION(macOS Mojave): move-by-word-visually-inline-block-positioned-element.html fails
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, koivisto, megan_gardner, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
Fixes the bug wenson_hsieh: review+

Ryosuke Niwa
Reported 2018-06-07 21:37:44 PDT
The layout test fails on macOS Mojave.
Attachments
WIP (1.25 KB, patch)
2018-06-07 21:38 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixes the bug (4.47 KB, patch)
2018-06-07 23:45 PDT, Ryosuke Niwa 		wenson_hsieh: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2018-06-07 21:38:11 PDT
Created attachment 342239 [details] WIP
Ryosuke Niwa
Comment 2 2018-06-07 23:45:03 PDT
Created attachment 342244 [details] Fixes the bug
Megan Gardner
Comment 3 2018-06-08 00:08:32 PDT
Wow, that’s one crazy bug! R=me, unofficially.
Ryosuke Niwa
Comment 4 2018-06-08 00:47:43 PDT
Comment on attachment 342244 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=342244&action=review > Source/WebCore/ChangeLog:32 > + wordBreakIteratorForMaxOffsetBoundary with visible position at offset 0 of "world", it thiss line and return nullptr. It should read "it skip this line and return nullptr."
Alexey Proskuryakov
Comment 5 2018-06-08 09:04:18 PDT
Great work! Is it possible to add a test that verifies this bug fix in a more straightforward manner, and maybe even extends testing in similar edge cases? Seems like we got lucky that a mostly unrelated test caught this.
Ryosuke Niwa
Comment 6 2018-06-08 13:06:04 PDT
(In reply to Alexey Proskuryakov from comment #5) > Great work! > > Is it possible to add a test that verifies this bug fix in a more > straightforward manner, and maybe even extends testing in similar edge > cases? Seems like we got lucky that a mostly unrelated test caught this. I wouldn't say we got lucky since the failing test basically tests every word movement across divs with variety position values in order to catch exactly like the bug like this. Having said that, I can add a new test which tests this exact failure although it would still require macOS Mojave to test it. I'm not certain adding a new mostly redundant test like that is useful though...
Alexey Proskuryakov
Comment 7 2018-06-08 13:29:58 PDT
Sounds like it would not be very useful given that.
Ryosuke Niwa
Comment 8 2018-06-08 13:31:03 PDT
(In reply to Alexey Proskuryakov from comment #7) > Sounds like it would not be very useful given that. Okay. I'll land the patch as is then.
Ryosuke Niwa
Comment 9 2018-06-08 13:36:11 PDT
Committed r232635: <https://trac.webkit.org/changeset/232635>
Radar WebKit Bug Importer
Comment 10 2018-06-08 13:37:22 PDT
<rdar://problem/40948830>
Note You need to log in before you can comment on or make changes to this bug.