Closed Bug 1649980 Opened 3 months ago Closed 3 months ago

Make `WSRunScanner` and `WSRunObject` stop taking DOM points at construction

Categories

(Core :: DOM: Editor, task)

task

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(22 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

WSRunScanner and WSRunObject take one or two EditorDOMPoints and initialize its members for the point. However, all users of the storing data uses new stack only class TextFragmentData to know what fragments there are. So, every such user should take the range instead of using stored range.

Severity: -- → S3

As coming patches, I believe that WSRunObject's job is, it keeps visible of white-spaces before and after modifying the DOM tree. E.g., if invisible leading/trailing white-spaces in block element may become visible, it deletes them. If visible white-space sequence is being split, it may replace collapsible ASCII white-space at the split point with NBSP if needed. So, perhaps, it should be renamed to WhiteSpaceVisibilityKeeper, but the name is too long and it becomes a class whose all methods are static. Therefore, the class names will be in HTMLEditor a lot...

I still don't have better name idea of WSRunScanner, though.

For making WSRunScanner::BoundaryData independent from WSRunScanner,
its initializer should be in the class itself as static factory methods.

Depends on D82295

This patch makes WSRunScanner have TextFragmentData const mTextFragmentData
instead of 2 BoundaryDatas, a NoBreakingSpaceData and a bool storing
whether it's preformatted or not.

Depends on D82694

They work with a TextFragmentData instance, and the following patches
require to run it without WSRunScanner/WSRunObject instances.
Therefore, this patch moves them into TextFragmentData.

Depends on D82695

It's called by WSRunObject::InsertText() and WSRunObject::InsertBreak() and
it has 2 parts. One is, considering whether previous char is an NBSP or not
and whether it should be replaced with an ASCII white-space if it's followed
by visible character. The other is, doing the replacement. The latter code
is enough simple. Therefore, we can copy them into the callers. Then,
we can move the check logic into TextFragmentData.

Depends on D82698

Same as the previous patch, it can be split to computation part and
modifying the DOM tree part. Then, the former can be in TextFragmentData
and the latter can be done by the caller which is only
WSRunObject::InsertText().

Depends on D82699

CreateVisibleWhiteSpacesData() is now called multiple times and maybe called
after the DOM tree is modified even though it's a bug. Therefore, we should
make it store first result and return its reference instead.

Depends on D82700

It's simpler to make WSRunScanner::InsertText() take insertion point.
Then, it can do its jobs with TextFragmentData instance(s).

Depends on D82701

Now, mScanEndPoint is not used. This patch removes it and clean up the
constructors of WSRunScanner and WSRunObject.

Depends on D82702

It's now can work with static helper methods and a TextFragmentData instance.
Therefore, this patch makes it a static method.

Note that it's always called with nsIEditor::eNone so that we can get rid of
the argument.

Depends on D82704

Now, it does not need to be a WSRunObject instance so that its wrapper to
create WSRunObject instance is not necessary.

Depends on D82709

Similar to the previous patch, WSRunObject::NormalizeWhiteSpacesAround() is
a wrapper to create WSRunObject instance for calling
NormalizeWhiteSpacesAtEndOf(), but it does not need to be WSRunObject's
instance. Therefore, we can merge them.

Note that this renames the merged method to NormalizeVisibleWhiteSpacesAt.

Depends on D82710

Now, all member methods of WSRunObject are static. So, it shouldn't
be able to instantiated.

Depends on D82711

Although the new name is long, but I have no better idea. The class's purpose
is to keep white-space visibility around modifying DOM position. Therefore,
I use "keeper" for the name.

Depends on D82712

Now, these classes are used only by TextFragmentData and they can be not
exposed. Therefore, we should hide them with making them private nested
classes of TextFragmentData.

Depends on D82713

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3e25637fd897
part 1: Move `WSRunScanner::mStart` and `WSRunScanner::mEnd` initializers into `BoundaryData` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4a06b8e95f27
part 2: Move `WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent()` to `HTMLEditUtils` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6de01deddd41
part 3: Make `WSRunScanner` store white-space sequence data with a `TextFragmentData` instance r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/88c47cd8a620
part 4: Implement `GetInclusiveNextEditableCharPoint()` and `GetPreviousEditableCharPoint()` in `TextFragmentData` instead of `WSRunScanner` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ade6312bf1c1
part 5: Implement `GetEndOfCollapsibleASCIIWhiteSpaces()` and `GetPreviousCharPointFromPointInText()` in `TextFragmentData` instead of `WSRunScanner` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6fedbc9d3130
part 6: Redesign `WSRunObject::MaybeReplacePreviousNBSPWithASCIIWhiteSpace()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5daee02c9dc7
part 7: Redesign `WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3309e46b2f22
part 8: Make `TextFragmentData::CreateVisibleWhiteSpacesData()` cache its result and returns reference instead of copy of the instance r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/daf96afe7740
part 9: Make `WSRunScanner::InsertText()` a static method r=m_kato
https://hg.mozilla.org/integration/autoland/rev/935a16d8abe6
part 10: Get rid of `WSRunScanner::mScanEndPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d359b5c68942
part 11: Make `WSRunObject::ReplaceASCIIWhiteSpacesWithOneNBSP()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/9e51d9c0ce8f
part 12: Make `WSRunObject::InsertBreak()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/906ef2c7cb40
part 13: Make `WSRunObject::PrepareToDeleteRangePriv()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/83f2c4696006
part 14: Make `WSRunObject::PrepareToSplitAcrossBlocksPriv()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/0b69c904f104
part 15: Make `WSRunObject::DeleteWSBackward()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/cb66c9898f9d
part 16: Make `WSRunObject::DeleteWSForward()` static r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ae9c2245690b
part 17: Merge `WSRunObject::DeleteInvisibleASCIIWhiteSpacesInternal()` to `WSRunObject::DeleteInvisibleASCIIWhiteSpaces()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/aaa0c6fdc568
part 18: Merge `WSRunObject::NormalizeWhiteSpacesAtEndOf()` to `WSRunObject::NormalizeWhiteSpacesAround()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/2845e156ce45
part 19: Make `WSRunObject` class not instantiated r=m_kato
https://hg.mozilla.org/integration/autoland/rev/bcef72851157
part 20: Rename `WSRunObject` to `WhiteSpaceVisibilityKeeper` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/977da8bc0c21
part 21: Move `BoundaryData` and `NoBreakingWhiteSpacesData` into `TextFragmentData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e3bfcfb8feb3
part 22: Make `VisibleWhiteSpacesData` store start point and end point with `EditorDOMPoint` r=m_kato
You need to log in before you can comment on or make changes to this bug.