Closed Bug 989012 Opened 6 years ago Closed 5 years ago

Cursor skips a character after going over images in contentEditable

Categories

(Core :: DOM: Selection, defect)

28 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: roan.kattouw, Assigned: ehsan)

References

Details

Attachments

(3 files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140317233501

Steps to reproduce:

Go to http://jsfiddle.net/VbL9g/1/ and step through each "Foo<img>Bar" sentence using the arrow keys.


Actual results:

For most images, the cursor is directly to the left of the image, pressing the right arrow key sends the cursor directly to the right of the image (from Foo|[image]Bar to Foo[image]|Bar). However, for the ones labeled "no src", the cursor goes one position too far (from Foo|[image]Bar to Foo[image]B|ar).

This behavior looks like an attempt to ignore invisible images, but it ignores images that have a visible alt text and doesn't ignore images that have their dimensions set to 0x0.


Expected results:

The behavior should be consistent :)

I would argue that invisible images should never be skipped (i.e. the cursor should always go from Foo|[image]Bar to Foo[image]|Bar), they're objects and they should occupy a cursor position even if they're not visible. This is how Chrome behaves. However, if Firefox decides that its behavior should be different from Chrome's and that invisible images should be skipped, it should at least correctly determine whether an image is invisible.
Confirmed with 2014-04-02-03-02-01-mozilla-central-firefox-31.0a1.en-US.linux-x86_64.
QA Whiteboard: [bugday-20140407]
Component: Untriaged → Editor
Product: Firefox → Core
Summary: Cursor skips over images in contentEditable → Cursor skips a character after going over images in contentEditable
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'll take a look at this...
Flags: needinfo?(ehsan)
I have fixes for the issues in comment 0.  These fixes enable us to skip over all kinds of images no matter what their src and alt attributes are the same way.  With these, we should behave the same as Chrome.
Assignee: nobody → ehsan
Component: Editor → Selection
Flags: needinfo?(ehsan)
The caret movement code already handles unselectable text frames if we
happen to land in the middle of one in nsTextFrame::PeekOffsetCharacter/Word.
However, when performing frame traversal to find the next frame to jump
to, we don't remember if we skipped over an unselectable frame, which causes
us to jump one offset too much when the caret is on the boundary of
selectable and unselectable content.  The test cases demonstrate the
scenario.  Note that an <img alt=foo> is implemented by adding a
generated content to the inline frame representing it, so as far as
the caret movement code is concerned, both test cases are treated similarly.
Attachment #8549702 - Flags: review?(roc)
We do not want to traverse inside native anonymous elements, but we
should still be able to skip over generated content, to avoid getting
stuck on such images.
Attachment #8549704 - Flags: review?(roc)
This fixes the test failure on try.  We want the behavior that part 1
implements only when moving the selection and not when extending it.
I will fold this into part 1 when landing.
Attachment #8549929 - Flags: review?(roc)
roc: ping?  I'd like to land this before I leave for vacation...
Flags: needinfo?(roc)
(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #10)
> Backed out because of Linux32 mochitest-a11y failure:
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/007fbd5bc4bd
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-
> inbound-linux/1422390992/mozilla-inbound_ubuntu32_vm_test-mochitest-other-
> bm05-tests1-linux32-build1648.txt.gz

Freaking implicit conversions :(

<https://dxr.mozilla.org/mozilla-central/source/accessible/generic/HyperTextAccessible.cpp#513>
Flags: needinfo?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/39c448409484
https://hg.mozilla.org/mozilla-central/rev/739b6b7ed6bc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Depends on: 1484593
You need to log in before you can comment on or make changes to this bug.