Closed Bug 644428 Opened 9 years ago Closed 9 years ago

cursor position on wyiswyg editors

Categories

(Core :: Editor, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: ebrahim, Assigned: ehsan)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0) Gecko/20110322 Firefox/4.0 Iceweasel/4.0
Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0) Gecko/20110322 Firefox/4.0 Iceweasel/4.0

i simplified a bug that i seen in wyiswyg editors using html5 contenteditable. it can reproduced with any html4 wyiswyg editors

Reproducible: Always

Steps to Reproduce:
1. put this in a html file <div contenteditable="true" style="border:solid 1px">a<span>b</span>c </div>
2. try remove characters by backspace slowly from end of it.
3. cursor will moved to next line!



i tested both linux and windows version of firefox4. i can see this problem on both of them
a fix:
3. after removing "b" from that text (you must start removing from end of it)  keyboards cursors indicator will be moved to nextline.
OS: Linux → All
Hardware: x86_64 → All
Component: General → Editor
Product: Firefox → Core
QA Contact: general → editor
Version: unspecified → Trunk
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/c0dbdafa583c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Firefox/4.0b8pre ID:20101104134944
Fails:
http://hg.mozilla.org/mozilla-central/rev/52a6a18eeb61
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Firefox/4.0b8pre ID:20101104165457
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c0dbdafa583c&tochange=52a6a18eeb61

In local build,
build from 217e60535a77 : fails
build from ed0befc22bb7 : fails
build from 8389e11781ed : works

Regressed between cset 8389e11781ed and ed0befc22bb7
Blocks: 389321
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Attached patch Patch (v1) (obsolete) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #522532 - Flags: review?(roc)
+  if (mRect.height == 0) {
+    // Empty inline frames will be pushed down in the line, so we need to
+    // account for that here.
+    baseline *= 0;

Why not just baseline = 0?

What if the height is > 0 but less than the font max-ascent? Can that ever happen? If not, we should assert it here.
Attached patch Patch (v2) (obsolete) — Splinter Review
(In reply to comment #4)
> +  if (mRect.height == 0) {
> +    // Empty inline frames will be pushed down in the line, so we need to
> +    // account for that here.
> +    baseline *= 0;
> 
> Why not just baseline = 0?

Oops, typo!

> What if the height is > 0 but less than the font max-ascent? Can that ever
> happen? If not, we should assert it here.

I don't think that should happen, so I don't think that we need to handle that.  But I agree that it's a very good idea to assert that here.
Attachment #522532 - Attachment is obsolete: true
Attachment #522532 - Flags: review?(roc)
Attachment #522737 - Flags: review?(roc)
Attached patch Patch (v3) (obsolete) — Splinter Review
Added some test files to the patch.  hg addremove should be automatic...
Attachment #522737 - Attachment is obsolete: true
Attachment #522737 - Flags: review?(roc)
Attachment #522747 - Flags: review?(roc)
Attached patch Patch (v4) (obsolete) — Splinter Review
The assertion message was correct, but I mistyped the comparison operator...  Sorry for so many iteration on this bug!
Attachment #522747 - Attachment is obsolete: true
Attachment #522891 - Flags: review?(roc)
http://hg.mozilla.org/mozilla-central/rev/db8e95fbcd39
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Thanks ehsan working well in night builds

I found other thing same this bug, if in empty dir="RTL" textarea you insert a latin char, in first inserting, cursor will moved to next line.

This can reproduce in both night builds and stable version.
Also I can reproduced that in other cases:

In normal textarea: (a persian char), space, backspace;
or (a persian char), backspace, (a persian char)

In RTL textarea: (a persian char), (latin char), backspace, (latin char)

And other states that I think can discovered.
(In reply to comment #9)
> Thanks ehsan working well in night builds
> 
> I found other thing same this bug, if in empty dir="RTL" textarea you insert a
> latin char, in first inserting, cursor will moved to next line.
> 
> This can reproduce in both night builds and stable version.

That's bug 646382.
Thanks!
Depends on: 647471
Reopening to implement bug 646382 comment 10.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #522891 - Attachment is obsolete: true
Attachment #524743 - Flags: review?(roc)
Pushed http://hg.mozilla.org/mozilla-central/rev/3c21288157a9
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 654421
You need to log in before you can comment on or make changes to this bug.