Closed Bug 645119 Opened 9 years ago Closed 9 years ago

Caret doesn't correctly move to the next line when also creating a new line in gmail

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

(Keywords: regression)

Attachments

(1 file)

I just noted this in a nightly built from <http://hg.mozilla.org/mozilla-central/rev/e11c2f95f781>.  In gmail, when replying to a text-only message, if you go to the end of a line and press Enter, the caret moves to the next line, but no new line gets drawn.  Switching tabs fixes the problem.

It's probably a regression from one of my recent patches.  I'm trying to find out which one now.
Actually it seems that this doesn't have anything to do with my changes.  Bisecting now...
OK, found the culprit:

ehsan@ehsan-Macmini:~/moz/src$ hg bis -b
The first bad revision is:
changeset:   63814:91604d53c7ce
user:        Simon Montagu <smontagu@smontagu.org>
date:        Thu Mar 24 11:28:45 2011 +0200
summary:     Bug 263359 part 5: optimization -- don't do bidi resolution if the text is monodirectional. r=roc

I'm going to back out bug 63814, because this looks really bad...
Blocks: 63814
I was obviously talking about bug 263359...
Blocks: 263359
No longer blocks: 63814
Locally backing out changeset 91604d53c7ce fixes the problem.
Looks like ehsan backed out the following changesets:
	91604d53c7ce	Simon Montagu — Bug 263359 part 5: optimization -- don't do bidi resolution if the text is monodirectional. r=roc
	56cc287f3860	Simon Montagu — Bug 263359 part 4: resolve paragraph on encountering line breaks in preformatted elements
	6c2c72be33e0	Simon Montagu — Tests for bug 263359
	c926f46f97ce	Simon Montagu — Bug 263359 part 3: resolve paragraph on encountering <br> or embedded block elements
	314216c47e6a	Simon Montagu — Tests for bug 229367 and bug 613157
	12faff7e29ea	Simon Montagu — Bug 263359 part 2: split nsBidiPresUtils::Resolve into Resolve and ResolveParagraph. r=roc
	a3fe678d8560	Simon Montagu — Bug 263359 part 1.5: remove nsDirectionalFrame.
	b218c8609794	Simon Montagu — Bug 263359 part 1: refactor bidi resolution code, combining InitLogicalArray and CreateBlockBuffer. r=roc
	24402af6330a	Simon Montagu — Debugging code for Bidi resolution. NPOTDB

in http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=fb0f7cc68d0e
(In reply to comment #5)
> Looks like ehsan backed out the following changesets:
>     91604d53c7ce    Simon Montagu — Bug 263359 part 5: optimization -- don't do
> bidi resolution if the text is monodirectional. r=roc
>     56cc287f3860    Simon Montagu — Bug 263359 part 4: resolve paragraph on
> encountering line breaks in preformatted elements
>     6c2c72be33e0    Simon Montagu — Tests for bug 263359
>     c926f46f97ce    Simon Montagu — Bug 263359 part 3: resolve paragraph on
> encountering <br> or embedded block elements
>     314216c47e6a    Simon Montagu — Tests for bug 229367 and bug 613157
>     12faff7e29ea    Simon Montagu — Bug 263359 part 2: split
> nsBidiPresUtils::Resolve into Resolve and ResolveParagraph. r=roc
>     a3fe678d8560    Simon Montagu — Bug 263359 part 1.5: remove
> nsDirectionalFrame.
>     b218c8609794    Simon Montagu — Bug 263359 part 1: refactor bidi resolution
> code, combining InitLogicalArray and CreateBlockBuffer. r=roc
>     24402af6330a    Simon Montagu — Debugging code for Bidi resolution. NPOTDB
> 
> in http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=fb0f7cc68d0e

Yes.  The backout fixes this bug.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
I tried to write a mochitest for this but it passes without the fix, even though a very similar mochitest in attachment 522788 [details] [diff] [review] fails as expected. Ehsan, any idea why?
Attachment #523275 - Flags: feedback?(ehsan)
(In reply to comment #7)
> Created attachment 523275 [details] [diff] [review]
> Attempted testcase
> 
> I tried to write a mochitest for this but it passes without the fix, even
> though a very similar mochitest in attachment 522788 [details] [diff] [review] fails as expected. Ehsan,
> any idea why?

Hmm, snapshotWindow causes a flush.  Would that be something which would fix the symptoms on its own?  If you just return instead of setting s2 or s3 for example, do you see the symptom on the screen?
(In reply to comment #8)
> If you just return instead of setting s2 or s3 for
> example, do you see the symptom on the screen?

I do not.
(In reply to comment #9)
> (In reply to comment #8)
> > If you just return instead of setting s2 or s3 for
> > example, do you see the symptom on the screen?
> 
> I do not.

Hmm, then something is fundamentally different in the test.  If you remove the synthesizeKey calls and enter the keys manually, does that reproduce the problem on the screen?  If it does, then what happens if you add a setTimeout(..., 0) after each synthesizeKey before doing the next one?
Attachment #523275 - Flags: feedback?(ehsan)
You need to log in before you can comment on or make changes to this bug.