Bug 53070 - Cannot extend or modify forward by word over a non-contenteditable region
Summary: Cannot extend or modify forward by word over a non-contenteditable region
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Benjamin (Ben) Kalman
Depends on:
Reported: 2011-01-24 23:44 PST by Benjamin (Ben) Kalman
Modified: 2011-02-17 00:07 PST (History)
6 users (show)

See Also:

Patch (9.21 KB, patch)
2011-01-24 23:55 PST, Benjamin (Ben) Kalman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin (Ben) Kalman 2011-01-24 23:44:34 PST
Cannot extend or modify forward by word over a non-contenteditable region
Comment 1 Benjamin (Ben) Kalman 2011-01-24 23:55:13 PST
Created attachment 80029 [details]
Comment 2 Ryosuke Niwa 2011-01-25 00:09:28 PST
Comment on attachment 80029 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=80029&action=review

> LayoutTests/editing/selection/extend-forward-by-word-over-non-editable.html:37
> +  getSelection().modify("extend", "forward", "word");
> +  if (getSelection().toString() === noneditableChild.textContent)
> +    log("PASS for " + bidiName);
> +  else
> +    log("FAIL for " + bidiName + ", selection is \"" + getSelection() + "\" but should be \"" + noneditableChild.textContent + "\"");

So when we extend selection by word, we want to extend beyond the word boundary when the word was not editable?  Does that much other browsers?  Is it documented anywhere?
Comment 3 Benjamin (Ben) Kalman 2011-01-25 00:14:09 PST
rniwa: I was in the middle of writing this essay when you made your comment.  I'll address it in here :-)


I will paste the explanation from the changelog here, and attempt to justify it:

Change all instances of honorEditableBoundaryAtOrAfter to honorEditableBoundaryAtOrBefore and vice versa in the functions which determine the end/start of words/lines/sentences in visible_units.cpp.

This fixes the bug where moving forwards by a word over a non-contenteditable region would place the cursor inside that region, and then get moved back to the start of the word due to honorEditableBoundaryAtOrBefore. The cursor is now moved to the end of the region (which is effectively a noop in this case).

There are a few things to note about this:

(1) Perhaps surprisingly (initially), all layout tests still pass, and the test case passes when previously it didn't.

(2) Layout test passing aside, it won't result in incorrect behaviour caused by the cursor being places in a non-editable region.  How could it?  The functions still prevent this, I've just reversed them.

(3) It also won't let the cursor "escape" a contenteditable region; the text iterators still respect contenteditable boundaries.

(4) The change has a nice sideeffect of treating a non-contenteditable region e.g. <span contenteditable=false>foo bar baz</span> as a word for the purposes of selection.  Currently, "foo bar baz" is treated as an atomic unit for the purposes of moving by characters (i.e. arrow keys), but not for words.  Now it does.

Furthermore, this is what other browsers do.  Or attempt to do.  Both firefox and IE have slight bugs in this area.  Given the dom "foo bar <s editable=false>baz qux> quux asdf"
 - firefox jumps from "bar" over the "baz qux" all the way to the "asdf".
 - IE jumps from "foo" over "bar baz qux" to the "quux".
Clearly each is buggy and aiming for the behaviour that this patch results in.  I don't know if it's documented anywhere.

(5) Even though the bug only affects moving by words, I changed all of them for consistency.
Comment 4 Ryosuke Niwa 2011-01-25 10:29:30 PST
Comment on attachment 80029 [details]

sounds good to me.  you might want to re-upload your patch since the patch doesn't seem to apply on TOT.
Comment 5 WebKit Commit Bot 2011-01-25 10:42:51 PST
Comment on attachment 80029 [details]

Clearing flags on attachment: 80029

Committed r76610: <http://trac.webkit.org/changeset/76610>
Comment 6 WebKit Commit Bot 2011-01-25 10:42:56 PST
All reviewed patches have been landed.  Closing bug.