Closed Bug 932370 Opened 6 years ago Closed Last year

Problems when entering a linebreak in the default Rich Text Editor of forums running XenForo software

Categories

(Firefox for Android :: Keyboards and IME, defect)

26 Branch
Other
Android
defect
Not set

Tracking

()

RESOLVED INACTIVE
Firefox 30

People

(Reporter: lmmozillabug, Assigned: jchen)

References

Details

Attachments

(3 files, 1 obsolete file)

A line break is created, but the cursor immediately returns to the space prior to the line break. From this point forward text is inputted as if in "insert mode", with all text appearing to the right of the cursor which no longer moves. "Insert mode" persists until page is refreshed or user switches to simplified text editor.

Using the Google Keyboard, keyboard suggestions are immediately entered following any keystroke. Though this suggestion behavior does not occur when using SwiftKey, the switch to "insert mode" following the pressing of the return key is consistent across keyboards.

Tested on Firefox v25, v26, and v28. All versions exhibit identical behavior. Problem does not occur in Chrome or Opera apps.
Version: Firefox 28 → Firefox 25
Summary: Cannot enter linebreak in XenForo forums' rich text editor (default) → Problems when entering linebreak in XenForo forums' rich text editor (default)
Summary: Problems when entering linebreak in XenForo forums' rich text editor (default) → Problems when entering linebreak in XenForo forums' default text editor
Thanks for the report. Can you provide a test URL?
XenForo has a forum on their website, but you'll need to create an account to access the reply box:

http://xenforo.com/community/forums/general-xenforo-discussion-and-feedback.45/
Summary: Problems when entering linebreak in XenForo forums' default text editor → Problems when entering a linebreak in the default Rich Text Editor of forums running XenForo software
Problem remains in versions 26.01, 27, 28.0a2, and 29.0a1.
Version: Firefox 25 → Firefox 26
Which device and Android version are you using?
Flags: needinfo?(lmmozillabug)
I am using a Sprint HTC One, running stock Android 4.3 (the latest available official release for my model).
Flags: needinfo?(lmmozillabug)
I just confirmed that the bug also occurs on an LG G Pad 8.3 Google Play Edition running Android 4.4.2.
Have you used the Android "adb" tool before? It'll be very helpful if you can provide the logcat output using this Nightly below, while you're trying to enter a line break. Thanks!

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nchen@mozilla.com-be395f8ddacd/try-android/fennec-29.0a1.en-US.android-arm.apk
Flags: needinfo?(lmmozillabug)
I started logcat output on my HTC One, tapped the text box to open the SwiftKey keyboard, and then input the following text one character at a time:

>test
>testtest
>test test
>test
>
>test
>testtest
>test test
>test

The resultant text in the text box was this:

>test test test test testtesttteststestet
>testtteststestetestt testtesttteststestet
>testtteststestetestttesttteststestet
>testtteststestet
>
>testtteststestet

When using the provided Nightly, I am seeing what appears to be identical behavior when using the SwiftKey keyboard as when using the Google Keyboard. Under Firefox v26, Swiftkey behavior differs and the following text results from the test input above:

>testtset
>tset tset
>tsettset
>tset
>
>tset
>tset tset
>tsettset
Flags: needinfo?(lmmozillabug)
Thank you very much! I'll look at the logcat and hopefully identify what's happening.
Assignee: nobody → nchen
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(nchen)
Does this Nightly improve things? http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nchen@mozilla.com-0f24721f46dd/try-android/fennec-29.0a1.en-US.android-arm.apk

Please attach the logcat as well. Thanks!
Flags: needinfo?(nchen) → needinfo?(lmmozillabug)
One problem is nsTextStateManager::ContentRemoved() doesn't work for deleted BR nodes. We measure the text length of the deleted BR node by calling GetFlatTextOffsetOfRange with an offset of 1, but that does not work. This patch adds a behavior to GetFlatTextOffsetOfRange: when the specified offset is -1, the function will measure to the end of the node, and will work for BR nodes.
Attachment #8383946 - Flags: review?(masayuki)
Comment on attachment 8383946 [details] [diff] [review]
Properly measure offsets for deleted BR nodes (v1)

I don't understand what is the cause actually. When this bug occurs, what are the aContainer, aChild and the childOffset value before a call of the second GetFlatTextOffsetOfRange()?

Anyway, I don't like the approach using "-1" for it because it may run the special case with a caller side's bug (e.g., passing 2 - 3).

And I'm not familiar with mutation observer's behavior, though. Does this work fine with remove text node or element in HTML editor?
This happens with a document like "<div>Test<br></div>". When the <br> node is deleted, nsTextStateManager::ContentRemoved is called with these arguments,

aDocument = doc
aContainer = div
aChild = br
aIndexInContainer = 1 (index 0 is text node)
aPreviousSibling = <text>

Now we want to notify widget of text change with arguments, start = 4, oldEnd = 5, newEnd = 4.

To get start and newEnd = 4, we call GetFlatTextOffsetOfRange with node = aContainer (div) and offset = aIndexInContainer (1).

To get oldEnd = 5, we need to find the length of the deleted aChild (br node):

* With the old code, we call GetFlatTextOffsetOfRange with node = aChild (br) and offset = 1. However, this does not work because <br> does not have a child with offset = 1, and the call fails.

* With the new code, we call GetFlatTextOffsetOfRange with node = aChild (br) and offset = -1. This works because we define offset = -1 as "end of the node", so the returned offset is the length of the node.
So, it sounds like that SetEnd(node, node->Length() + 1) always fails.

If so, in GetFlatTextOffsetOfRange(), isn't following code enough to fix this bug? Other callers might have same bug.

   if (aNode->Length() >= aNodeOffset) {
     prev->SetEnd(startDOMNode, aNodeOffset);
     iter->Init(prev);
   } else if (aNode != static_cast<nsINode*>(aRootContent)) {
     prev->SetEndAfter(startDOMNode);
     iter->Init(prev);
   } else {
     iter->Init(aRootContent);
   }
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #14)
> So, it sounds like that SetEnd(node, node->Length() + 1) always fails.
> 
> If so, in GetFlatTextOffsetOfRange(), isn't following code enough to fix
> this bug? Other callers might have same bug.

Yup, that works too.
Attachment #8383946 - Attachment is obsolete: true
Attachment #8383946 - Flags: review?(masayuki)
Attachment #8388988 - Flags: review?(masayuki)
Attachment #8388988 - Flags: review?(masayuki) → review+
FYI: the file name is now dom/events/ContentEventHandler.cpp. Please change the file name in the diff file before landing onto m-i.
https://hg.mozilla.org/mozilla-central/rev/c4f51e54c14d
https://hg.mozilla.org/mozilla-central/rev/f98f596ad056
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
I apologize for the late response. This bug is still present in versions 28.0, 29.0a2, and 31.0a1. The last APK link above is dead; would you like logcat output from the Nightly 31.0a1, or should I test with a different build?
Status: RESOLVED → REOPENED
Flags: needinfo?(nchen)
Resolution: FIXED → ---
(In reply to lmmozillabug from comment #20)
> I apologize for the late response. This bug is still present in versions
> 28.0, 29.0a2, and 31.0a1. The last APK link above is dead; would you like
> logcat output from the Nightly 31.0a1, or should I test with a different
> build?

No worries! Can you try this build and post the logs?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nchen@mozilla.com-6233fe5e3a76/try-android/fennec-31.0a1.en-US.android-arm.apk
Flags: needinfo?(nchen)
Here is the requested log. I used the same test:

Expected output:
>test
>testtest
>test test
>test
>
>test
>testtest
>test test
>test

Actual output:
>test testtesttteststestet testtesttteststestet
>testtteststestetestt testtteststestetestttesttteststestet
>testtteststestetestttesttteststestet
>testtteststestet
>
>testtteststestet
Flags: needinfo?(lmmozillabug)
Status: REOPENED → RESOLVED
Closed: 6 years agoLast year
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.