Better work around for Blink selection issues.

Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as follows. Walk up to the parent of the static text, compute the length of all of the text descendants up to that static text, and finally add any remaining offsets into the static text in question.

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as expected but the anchor node is set to the static text for the link's static text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't uncovered yet.

BUG=626385
TEST=chromevox_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/d14eace0f228ce66113791ad781640e311d0e549
Cr-Commit-Position: refs/heads/master@{#406413}

[David Tseng, 2016-07-15 18:43:51]

Description was changed from

==========
Better work around for Blink selection issues.
==========

to

==========
Better work around for Blink selection issues.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-07-15 18:50:59]

Description was changed from

==========
Better work around for Blink selection issues.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text up to that static text, and finally add any remaining offsets into the
static text in question. It doesn't appear to matter what the client passes as
the anchor/focus object.

The second point is surprising and should probably be fixed at some point.



BUG=626385
TEST=chromevox_tests
==========

[David Tseng, 2016-07-15 18:50:59]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org, nektar@chromium.org

[David Tseng, 2016-07-15 19:00:06]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text up to that static text, and finally add any remaining offsets into the
static text in question. It doesn't appear to matter what the client passes as
the anchor/focus object.

The second point is surprising and should probably be fixed at some point.



BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text up to that static text, and finally add any remaining offsets into the
static text in question. It doesn't appear to matter what the client passes as
the anchor/focus object.

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

[David Tseng, 2016-07-15 19:01:09]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text up to that static text, and finally add any remaining offsets into the
static text in question. It doesn't appear to matter what the client passes as
the anchor/focus object.

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. It doesn't appear to matter what the
client passes as the anchor/focus object.

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

[David Tseng, 2016-07-15 19:02:45]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. It doesn't appear to matter what the
client passes as the anchor/focus object.

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

[David Tseng, 2016-07-15 19:03:44]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

point.
This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

[David Tseng, 2016-07-15 19:17:08]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text
child 2: link
child 3: static text

exhibits this behavior. Furthermore, the link itself which contains a static
text, is starts with a 0-offset (following the same rules above).

This is surprising and should probably be fixed or called out very clearly
everywhere.

BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static
text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as
expected but the anchor node is set to the static text for the link's static
text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't
uncovered yet.

BUG=626385
TEST=chromevox_tests
==========

[chromium-reviews, 2016-07-15 19:51:06]

+ var texts = parent.findAll({role: RoleType.staticText});
What about line break roles?

> + // Only allow selections inside of the same web tree.
> + if (startNode.root &&
> + startNode.root.role == RoleType.rootWebArea &&
> + startNode.root === endNode.root) {
> var startIndex = this.start.selectionIndex_;
> No selection allowed across cross-process iframes?


> +TEST_F('CursorsTest', 'InlineElementOffset', function() {
> + this.runWithLoadedTree(function() {/*!
> + <p>This is a<a href="#g">test</a>of selection</p>
> + */}, function(root) {
> + root.addEventListener('textSelectionChanged', 
> this.newCallback(function(evt) {
> + // This is a little unexpected though not really incorrect; Ctrl+C 
> works.
> + assertEquals(testNode, root.anchorObject);
> + assertEquals(ofSelectionNode, root.focusObject);
> + assertEquals(4, root.anchorOffset);
> + assertEquals(1, root.focusOffset);
This is certainly unexpected and I don't get why it is correct. We 
haven't selected anything from inside the link.
I understand that conceptually it might make sense, but it's confusing.
What's the cause?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-07-15 19:51:28]

lgtm

[David Tseng, 2016-07-15 19:59:31]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 20:00:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 21:16:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 21:16:39]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2016-07-19 20:41:03]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 20:41:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Tseng, 2016-07-19 21:43:03]

On Fri, Jul 15, 2016 at 12:51 PM, Nektarios Paisios <nektar@google.com>
wrote:

> + var texts = parent.findAll({role: RoleType.staticText});
> What about line break roles?
>
> Good catch; added (and tested).

 N

> + // Only allow selections inside of the same web tree.
> + if (startNode.root &&
> + startNode.root.role == RoleType.rootWebArea &&
> + startNode.root === endNode.root) {
> var startIndex = this.start.selectionIndex_;
> No selection allowed across cross-process iframes?
>
>
> Nope; they'ed be separate ipc's. We could divide up the selection into
each subroot and forward those requests to each renderer. Left for another
cl.


>
> +TEST_F('CursorsTest', 'InlineElementOffset', function() {
> + this.runWithLoadedTree(function() {/*!
> + <p>This is a<a href="#g">test</a>of selection</p>
> + */}, function(root) {
> + root.addEventListener('textSelectionChanged',
> this.newCallback(function(evt) {
> + // This is a little unexpected though not really incorrect; Ctrl+C works.
> + assertEquals(testNode, root.anchorObject);
> + assertEquals(ofSelectionNode, root.focusObject);
> + assertEquals(4, root.anchorOffset);
> + assertEquals(1, root.focusOffset);
>
> This is certainly unexpected and I don't get why it is correct. We haven't
> selected anything from inside the link.
> I understand that conceptually it might make sense, but it's confusing.
> What's the cause?
> I haven't dug into it. Load balancing and leaving to dmazzoni.
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2016-07-19 21:56:38]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static
text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as
expected but the anchor node is set to the static text for the link's static
text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't
uncovered yet.

BUG=626385
TEST=chromevox_tests
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static
text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as
expected but the anchor node is set to the static text for the link's static
text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't
uncovered yet.

BUG=626385
TEST=chromevox_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-07-19 21:56:44]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-07-19 21:56:45]

The patchset sent to the CQ was uploaded after l-g-t-m from nektar@chromium.org
Link to the patchset: https://codereview.chromium.org/2153213002/#ps20001
(title: "Handle line breaks.")

[commit-bot: I haz the power, 2016-07-19 21:57:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 23:36:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-19 23:36:46]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-19 23:38:23]

Description was changed from

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question. 

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static
text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as
expected but the anchor node is set to the static text for the link's static
text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't
uncovered yet.

BUG=626385
TEST=chromevox_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Better work around for Blink selection issues.

In the interest of getting something that works, conform to the way selections
work in the Blink accessibility tree.

For reference, the way it behaves is as follows:
- on non-static text nodes, the index is a child index (0-based).
- if the node is a static text node, the client must compute an offset as
follows. Walk up to the parent of the static text, compute the length of all of
the text descendants up to that static text, and finally add any remaining
offsets into the static text in question.

For example, the paragraph containing:
child 1: static text name=this is a
child 2: link child rstatic text name=test
child 3: static text name=of selection

exhibits this behavior. Furthermore, the link itself which contains a static
text, starts with a 0-offset (following the same rules above).

- after making a selection, the offsets behave sort of more like DOM indexes.

For example, the above snippet when selecting offsets 13/14, gives back 'o' as
expected but the anchor node is set to the static text for the link's static
text at offset 4. The focus object is set to 1 for the last static text.

This is surprising and probably has a lot of weird corner cases I haven't
uncovered yet.

BUG=626385
TEST=chromevox_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/d14eace0f228ce66113791ad781640e311d0e549
Cr-Commit-Position: refs/heads/master@{#406413}
==========

[commit-bot: I haz the power, 2016-07-19 23:38:24]

Patchset 2 (id:??) landed as
https://crrev.com/d14eace0f228ce66113791ad781640e311d0e549
Cr-Commit-Position: refs/heads/master@{#406413}