989012 - Cursor skips a character after going over images in contentEditable

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Roan Kattouw]

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.

Comment 1

[[:Aleksej]]

Confirmed with 2014-04-02-03-02-01-mozilla-central-firefox-31.0a1.en-US.linux-x86_64.

Comment 2

[(no longer active)]

I'll take a look at this...

Comment 3

[(no longer active)]

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.

Comment 4

[(no longer active)]

Attachment: Part 1: Stop after passing over a non-selectable frame if one is found during the frame traversal

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.

Comment 5

[(no longer active)]

Attachment: Part 2: Don't get stuck when seeing an image without a src or alt attribute

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.

Comment 6

[(no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=715d199752e0

Comment 7

[(no longer active)]

Attachment: follow-up: Do not attempt to stop after a non-selectable frame when extending the selection

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.

Comment 8

[(no longer active)]

roc: ping?  I'd like to land this before I leave for vacation...

Comment 9

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22fb05349a10
https://hg.mozilla.org/integration/mozilla-inbound/rev/9698f6f3f72e

Comment 10

[(no longer active)]

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

Comment 11

[(no longer active)]

(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>

Comment 12

[(no longer active)]

Try again:

https://hg.mozilla.org/integration/mozilla-inbound/rev/39c448409484
https://hg.mozilla.org/integration/mozilla-inbound/rev/739b6b7ed6bc

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/39c448409484
https://hg.mozilla.org/mozilla-central/rev/739b6b7ed6bc