If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

execCommand("defaultparagraphseparator", false, "p"); sometimes leads to <enter> being ignored.

RESOLVED FIXED in Firefox 55

Status

()

Core
Editor
P1
critical
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: ayg)

Tracking

(Blocks: 1 bug, {regression})

unspecified
mozilla55
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

6 months ago
Created attachment 8854803 [details]
Simple HTML page showing the problem

+++ This bug was initially created as a clone of Bug #1297414 +++

See attached HTML document.

Instructions:
1) Click into the gap between the two blockquotes.
2) Hit <enter>
3) Hit <backspace>
4) Hit <enter> - Nothing happens.
Assignee: nobody → ayg
Status: NEW → ASSIGNED
More minimal test-case:

data:text/html,<!doctype html>
<div contenteditable><div>x</div><br></div>
<script>
getSelection().collapse(document.body.firstChild, 1);
document.body.firstChild.firstChild.focus();
document.execCommand("insertparagraph");
document.body.textContent = document.body.firstChild.innerHTML;
</script>

It should really be <div>x</div><div><br></div><div><br></div>, but if it were <div>x</div><br><br> it would at least work.  But it's <div>x</div><br>, which is no good at all.
Comment hidden (mozreview-request)
Nice, this was actually already tested by wpt.

Thanks a lot for the quick and well-written bug report, Jorg!
(Reporter)

Comment 4

6 months ago
(In reply to Aryeh Gregor (:ayg) (working March 28-April 26) from comment #1)
> More minimal test-case:
You always have simpler test cases, but mine come from real life ;-)

(In reply to Aryeh Gregor (:ayg) (working March 28-April 26) from comment #3)
> Thanks a lot for the quick and well-written bug report, Jorg!
You're welcome, thanks for fixing it quickly. I'll try it out in TB when I've fixed the lasted bustage that came our way from M-C.

Quoting from bug 1334771 comment #73:
I know that {some|most} M-C developers are not friends of Thunderbird. However, the TB team, despite being a pack of cats, have an excellent skill set and we are the secondary M-C testing brigade (bug 1328847 comment #38 "more examples of c-c usefully expanding the test coverage of m-c ;)"), because, to use the analogy, we are the kernel module developers and tread where no one else sets their foot. I could give you so many examples of TB people yelling when something wasn't right in FF.

As you know, TB has 25 million users, and all they ever do all day is hammer the M-C editor ;-)
(Reporter)

Comment 5

6 months ago
Aryeh, thanks for the fix, that fixes the issue observed in TB when splitting blockquotes, too. Much appreciated.
Duplicate of this bug: 1353913
This breaks Gmail, so landing a fix quickly would be good.  :)  This also proves that Gmail does use our line-breaking behavior!
Severity: normal → critical

Updated

6 months ago
status-firefox54: --- → unaffected
status-firefox55: --- → affected
Keywords: regression
Priority: P3 → P1

Comment 8

6 months ago
It seems it breaks some tests?
And there is no reviewer requested.

Comment 9

6 months ago
Sorry, I confused changes in insertparagraph.html.ini with test failures ;)
(Reporter)

Comment 10

6 months ago
Comment on attachment 8854971 [details]
Bug 1353695 - Sometimes Enter is ignored in editor;

Hmm, I don't know how review board works, but as Aceman pointed out, apparently no review requested. Sorry for the interference.
Attachment #8854971 - Flags: review?(masayuki)

Comment 11

6 months ago
mozreview-review
Comment on attachment 8854971 [details]
Bug 1353695 - Sometimes Enter is ignored in editor;

https://reviewboard.mozilla.org/r/126886/#review130738

::: editor/libeditor/HTMLEditRules.cpp:1575
(Diff revision 1)
> +    NS_ENSURE_TRUE(aSelection.GetRangeAt(0) &&
> +                   aSelection.GetRangeAt(0)->GetStartParent(),
> +                   NS_ERROR_FAILURE);

I'd like you to use:

if (NS_WARN_IF(!aSelection.GetRangeAt(0) &&
               !aSelection.GetRangeAt(0)->GetStartParent())) {
  return NS_ERROR_FAILURE;
}

style here.
Attachment #8854971 - Flags: review?(masayuki) → review+

Updated

6 months ago
Blocks: 1354931
Comment hidden (mozreview-request)

Comment 13

5 months ago
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/autoland/rev/f25386cf25ad
Sometimes Enter is ignored in editor; r=masayuki

Comment 14

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f25386cf25ad
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox53: --- → unaffected
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.