WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(84.12 KB, patch)
2020-03-18 18:31 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(95.00 KB, patch)
2020-03-21 14:47 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(94.99 KB, patch)
2020-03-21 15:57 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(96.75 KB, patch)
2020-03-22 12:46 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(83.95 KB, patch)
2020-03-22 14:14 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(96.57 KB, patch)
2020-03-22 14:18 PDT
,
Darin Adler
koivisto
: review+
Details
Formatted Diff
Diff
Patch
(97.28 KB, patch)
2020-03-23 12:43 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2020-03-17 18:49:28 PDT
Comment hidden (obsolete)
Created
attachment 393808
[details]
Patch
Darin Adler
Comment 2
2020-03-18 18:31:03 PDT
Comment hidden (obsolete)
Created
attachment 393931
[details]
Patch
Darin Adler
Comment 3
2020-03-21 14:47:25 PDT
Comment hidden (obsolete)
Created
attachment 394179
[details]
Patch
Darin Adler
Comment 4
2020-03-21 15:57:04 PDT
Comment hidden (obsolete)
Created
attachment 394182
[details]
Patch
Darin Adler
Comment 5
2020-03-22 12:46:33 PDT
Comment hidden (obsolete)
Created
attachment 394226
[details]
Patch
Darin Adler
Comment 6
2020-03-22 13:59:57 PDT
Comment hidden (obsolete)
Comment on
attachment 394226
[details]
Patch WPE build still failing
Darin Adler
Comment 7
2020-03-22 14:14:27 PDT
Comment hidden (obsolete)
Created
attachment 394230
[details]
Patch
Darin Adler
Comment 8
2020-03-22 14:18:14 PDT
Created
attachment 394231
[details]
Patch
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
Created
attachment 394289
[details]
Patch
Darin Adler
Comment 15
2020-03-23 13:50:22 PDT
Committed
r258871
: <
https://trac.webkit.org/changeset/258871
>
Radar WebKit Bug Importer
Comment 16
2020-03-23 13:51:15 PDT
<
rdar://problem/60790089
>
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.
Top of Page
Format For Printing
XML
Clone This Bug