Make `OffsetEntry` in `TextServicesDocument.cpp` clearer
Categories
(Core :: Spelling checker, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox92 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
(Blocks 1 open bug)
Details
Attachments
(18 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
Bug 1718924 - part 12: Move `TextServicesDocument::FindWordBounds()` to `OffsetEntryArray` r=m_kato!
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
Bug 1718924 - part 14: Move `TextServicesDocument::SplitOffsetEntry` to `OffsetEntryArray` r=m_kato!
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
It seems that it treats mainly a text node in various places, but it's not
guaranteed by any variable declarations. So, first of all, I'd like to make
it clearer.
TextServicesDocument::IsTextNode()
isn't necessary because nsINode::IsText()
is enough useful. And AsText()
should be zero cost at runtime. So, in blocks
which guarantee specific content node is a text node, this patch appends
AsText()
for making the code clearer.
Assignee | ||
Comment 2•3 years ago
|
||
Now, mNode
is always a text node, and it may store across "can run script"
boundaries. So, it should be OwningNonNull<Text>
.
Depends on D119148
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D119149
Assignee | ||
Comment 4•3 years ago
|
||
Depends on D119150
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D119151
Assignee | ||
Comment 6•3 years ago
|
||
Depends on D119152
Assignee | ||
Comment 7•3 years ago
|
||
Depends on D119153
Assignee | ||
Comment 8•3 years ago
|
||
Depends on D119154
Assignee | ||
Comment 9•3 years ago
|
||
Now, the meaning of OffsetEntry
is clear. Therefore, this patch adds comment
explaining the class and its members.
Then, the meaning of TextServicesDocument::mSelStartOffset
and
TextServicesDocument::mSelEndOffset
becomes clearer since they are used to
create OffsetEntry
instances. Therefore, this patch renames them.
Depends on D119155
Assignee | ||
Comment 10•3 years ago
|
||
Now, it stores dom::Text
with OwningNonNull
. So, once it's leaked, it
wastes a lot of memory spaces. Therefore, we should make mOffsetTable
store UniquePtr<OffsetEntry>
instead of OffsetEntry*
.
Depends on D119156
Assignee | ||
Comment 11•3 years ago
|
||
There are some methods in TextServicesDocument
which work only with
TextServicesDocument::mOffsetTable
. Once we move such methods to custom
class of nsTArray<UniquePtr<OffsetEntry>>
, we can make TextServicesDocument
simpler.
Depends on D119157
Assignee | ||
Comment 12•3 years ago
|
||
Depends on D119158
Assignee | ||
Comment 13•3 years ago
|
||
Depends on D119159
Assignee | ||
Comment 14•3 years ago
|
||
Depends on D119160
Assignee | ||
Comment 15•3 years ago
|
||
They are indices of OffsetEntryArray
. Therefore, they should be managed in it.
Depends on D119161
Assignee | ||
Comment 16•3 years ago
|
||
Depends on D119162
Assignee | ||
Comment 17•3 years ago
|
||
Depends on D119163
Assignee | ||
Comment 18•3 years ago
|
||
Depends on D119164
Comment 19•3 years ago
|
||
Comment 20•3 years ago
|
||
Updated•3 years ago
|
Comment 21•3 years ago
|
||
Comment 22•3 years ago
|
||
Comment 23•3 years ago
|
||
Comment 24•3 years ago
|
||
Comment 25•3 years ago
|
||
Comment 26•3 years ago
|
||
Comment 27•3 years ago
|
||
Comment 28•3 years ago
|
||
Comment 29•3 years ago
|
||
Comment 30•3 years ago
|
||
Comment 31•3 years ago
|
||
Comment 32•3 years ago
|
||
Comment 33•3 years ago
|
||
Comment 34•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Comment 35•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ade9a3381ccb
https://hg.mozilla.org/mozilla-central/rev/58608619e491
https://hg.mozilla.org/mozilla-central/rev/3857f3a91edb
https://hg.mozilla.org/mozilla-central/rev/3e43427ec342
https://hg.mozilla.org/mozilla-central/rev/87409043c8f8
https://hg.mozilla.org/mozilla-central/rev/8d3b1cd13597
https://hg.mozilla.org/mozilla-central/rev/790a5005cdbe
https://hg.mozilla.org/mozilla-central/rev/5a72fb7d3da4
https://hg.mozilla.org/mozilla-central/rev/3ba594b4b02d
https://hg.mozilla.org/mozilla-central/rev/42ff08e8e796
https://hg.mozilla.org/mozilla-central/rev/77740303aae0
https://hg.mozilla.org/mozilla-central/rev/622d9feefc23
https://hg.mozilla.org/mozilla-central/rev/0bffa7979736
https://hg.mozilla.org/mozilla-central/rev/3ffbb88fe641
https://hg.mozilla.org/mozilla-central/rev/b771c6d7f21a
https://hg.mozilla.org/mozilla-central/rev/da317e009565
https://hg.mozilla.org/mozilla-central/rev/1232d34c7296
https://hg.mozilla.org/mozilla-central/rev/6af3e577108b
Description
•