1725930 - `TextServicesDocument::HasSameBlockNodeParent` refers same text node to check 2 text nodes are in same block element

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P2)

Description

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

https://searchfox.org/mozilla-central/rev/d3683dbb252506400c71256ef3994cdbdfb71ada/editor/spellchecker/TextServicesDocument.cpp#1592-1593

bool TextServicesDocument::HasSameBlockNodeParent(Text& aTextNode1,
                                                  Text& aTextNode2) {
  nsIContent* container1 = aTextNode1.GetParent();
  nsIContent* container2 = aTextNode1.GetParent();

Could be a cause of bug 1721801, but not sure.

Comment 1

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

Attachment: Bug 1725930 - Make `TextServicesDocument::HasSameBlockNodeParent` refer 2nd text node as expected r=m_kato!

This is just a simple mistake, but I'm not sure how this break the callers.
Therefore, I don't have a testcase for this.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/708126343f0e
Make `TextServicesDocument::HasSameBlockNodeParent` refer 2nd text node as expected r=m_kato

Comment 3

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

Comment on attachment 9236415 [details]
Bug 1725930 - Make TextServicesDocument::HasSameBlockNodeParent refer 2nd text node as expected r=m_kato!

Beta/Release Uplift Approval Request

• User impact if declined: TextSevicesDocument which collects text in a block containing caret unexpectedly keeps collecting text in different blocks too. Even if this does not show any regression visually, this could cause bad performance or hunging up.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I'm not sure how this method work, however, the fix is simplest and obviously correct.
• String changes made/needed:

Comment 4

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/708126343f0e

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9236415 [details]
Bug 1725930 - Make TextServicesDocument::HasSameBlockNodeParent refer 2nd text node as expected r=m_kato!

Approved for 92.0b5.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/06aab3ee2ede