RESOLVED FIXED 209207
Change TextIterator::rangeLength to not require a live range
https://bugs.webkit.org/show_bug.cgi?id=209207
Summary Change TextIterator::rangeLength to not require a live range
Darin Adler
Reported 2020-03-17 18:13:59 PDT
Change TextIterator::rangeLength to not require a live range
Attachments
Patch (81.90 KB, patch)
2020-03-17 18:49 PDT, Darin Adler
no flags
Patch (84.12 KB, patch)
2020-03-18 18:31 PDT, Darin Adler
no flags
Patch (95.00 KB, patch)
2020-03-21 14:47 PDT, Darin Adler
no flags
Patch (94.99 KB, patch)
2020-03-21 15:57 PDT, Darin Adler
no flags
Patch (96.75 KB, patch)
2020-03-22 12:46 PDT, Darin Adler
no flags
Patch (83.95 KB, patch)
2020-03-22 14:14 PDT, Darin Adler
no flags
Patch (96.57 KB, patch)
2020-03-22 14:18 PDT, Darin Adler
koivisto: review+
Patch (97.28 KB, patch)
2020-03-23 12:43 PDT, Darin Adler
no flags
Darin Adler
Comment 1 2020-03-17 18:49:28 PDT Comment hidden (obsolete)
Darin Adler
Comment 2 2020-03-18 18:31:03 PDT Comment hidden (obsolete)
Darin Adler
Comment 3 2020-03-21 14:47:25 PDT Comment hidden (obsolete)
Darin Adler
Comment 4 2020-03-21 15:57:04 PDT Comment hidden (obsolete)
Darin Adler
Comment 5 2020-03-22 12:46:33 PDT Comment hidden (obsolete)
Darin Adler
Comment 6 2020-03-22 13:59:57 PDT Comment hidden (obsolete)
Darin Adler
Comment 7 2020-03-22 14:14:27 PDT Comment hidden (obsolete)
Darin Adler
Comment 8 2020-03-22 14:18:14 PDT
Darin Adler
Comment 9 2020-03-22 17:06:36 PDT
OK, all tests passing. Ready for review now.
Darin Adler
Comment 10 2020-03-22 17:07:08 PDT
Next step on moving off of live ranges.
Antti Koivisto
Comment 11 2020-03-23 05:27:19 PDT
Comment on attachment 394231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394231&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:1946 > - auto searchRange = Range::create(m_document, startPosition, endPosition); > - if (searchRange->collapsed()) > + SimpleRange searchRange { *makeBoundaryPoint(startPosition), *makeBoundaryPoint(endPosition) }; I sort of like auto searchRange = SimpleRange { *makeBoundaryPoint(startPosition), *makeBoundaryPoint(endPosition) }; for these. It is longer but I feel it reads better. It is also allows replacing constructor call with a function call (or opposite, like here) with minimal changes. > Source/WebCore/editing/TextIterator.h:299 > +WEBCORE_EXPORT CharacterCount characterCount(const SimpleRange&, bool spacesForReplacedElements = false); The boolean is rather magical in the call sites. Please add enum.
Darin Adler
Comment 12 2020-03-23 09:03:49 PDT
Comment on attachment 394231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394231&action=review >> Source/WebCore/accessibility/AXObjectCache.cpp:1946 >> + SimpleRange searchRange { *makeBoundaryPoint(startPosition), *makeBoundaryPoint(endPosition) }; > > I sort of like > > auto searchRange = SimpleRange { *makeBoundaryPoint(startPosition), *makeBoundaryPoint(endPosition) }; > > for these. It is longer but I feel it reads better. It is also allows replacing constructor call with a function call (or opposite, like here) with minimal changes. OK. That does seem better.
Darin Adler
Comment 13 2020-03-23 09:04:33 PDT
Comment on attachment 394231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394231&action=review >> Source/WebCore/editing/TextIterator.h:299 >> +WEBCORE_EXPORT CharacterCount characterCount(const SimpleRange&, bool spacesForReplacedElements = false); > > The boolean is rather magical in the call sites. Please add enum. Will do. Might want to do an OptionSet later, but for now an enum class should do.
Darin Adler
Comment 14 2020-03-23 12:43:29 PDT
Darin Adler
Comment 15 2020-03-23 13:50:22 PDT
Radar WebKit Bug Importer
Comment 16 2020-03-23 13:51:15 PDT
Diego Pino
Comment 17 2020-03-24 02:12:00 PDT
GTK test bot is exiting early due to too many crashes after this patch https://bugs.webkit.org/show_bug.cgi?id=209467
Note You need to log in before you can comment on or make changes to this bug.