1445935 - leftCite in HTMLEditRules::SplitMailCites() isn't set to left node of the split node

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://searchfox.org/mozilla-central/source/editor/libeditor/HTMLEditRules.cpp#1971,1973,1982,1989,2034,2036,2042
In this revision, leftCite is set to the left node of split node. However, now, leftCite is never set to non-nullptr:
https://searchfox.org/mozilla-central/source/editor/libeditor/HTMLEditRules.cpp#1992-1995,2006-2008,2017,2077,2079,2085

So, left node of the split node won't be removed even if it's unnecessary.

I found this when I work on another bug. So, I don't know how to meet actual symptom.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2eb9d193175623c07ac35cc93839d8962daa06e7

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1445935 - HTMLEditRules::SplitMailCites() remove left node of split node if it becomes empty

Before bug 1413181, HTMLEditRules::SplitMailCites() receives left node of
split node with |leftCite| from SplitNodeDeep().  However, the out argument
was removed by the bug and now, it can be retrieved with
SplitNodeResult::GetPreviousNode().  Its result is used for creating <br>
element etc, but not set to |leftCite| but HTMLEditRules::SplitMailCites()
still checks if |leftCite| is empty to remove it from the DOM tree.

So, now, both |leftCite| and |rightCite| are not necessary and
HTMLEditRules::SplitMailCites() should use |previousNodeOfSplitPoint|
instead of |leftCite|.

Review commit: https://reviewboard.mozilla.org/r/228038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228038/

Comment 3

[Makoto Kato [:m_kato]]

Comment on attachment 8959174 [details]
Bug 1445935 - HTMLEditRules::SplitMailCites() remove left node of split node if it becomes empty

https://reviewboard.mozilla.org/r/228038/#review234058

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce9c476f9dfc
HTMLEditRules::SplitMailCites() remove left node of split node if it becomes empty r=m_kato

Comment 5

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/ce9c476f9dfc

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8959174 [details]
Bug 1445935 - HTMLEditRules::SplitMailCites() remove left node of split node if it becomes empty

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1413181.

[User impact if declined]:
Nothing for Firefox users, this path runs only on mail composer except automated tests.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
No.

[Needs manual test from QE? If yes, steps to reproduce]: 
No.

[List of other uplifts needed for the feature/fix]:
None.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
I just forgot to do this in bug 1413181. Therefore, this patch just gets back the old behavior, it removes unnecessary (empty) <blockquote> element when pressing enter or something.

[String changes made/needed]:
No.

Comment 7

[Julien Cristau [:jcristau]]

Comment on attachment 8959174 [details]
Bug 1445935 - HTMLEditRules::SplitMailCites() remove left node of split node if it becomes empty

editor regression fix, beta60+

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b3e6be69e432