Bug 60211 – Introduce layoutTestController.currentSelection(), a way to non-invasively query current selection.

• Component: New Bugs
• Status: RESOLVED FIXED

[Dimitri Glazkov (Google)]

Introduce layoutTestController.currentSelection(), a way to non-invasively query current selection.

[Dimitri Glazkov (Google)]

Created attachment 92316 [details]
To bake on EWS.

[Early Warning System Bot]

Attachment 92316 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8558635

[WebKit Review Bot]

Attachment 92316 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8571199

[Ryosuke Niwa]

Comment on attachment 92316 [details]
To bake on EWS.

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

> Source/WebCore/dom/Position.cpp:1240
> +static String nodeAsJSONString(Node* node)
> +{
> +    String result = "{ ";
> +    // { name|value, ancestors }
> +    if (node->isElementNode() || node->isDocumentNode())
> +        result += "\"name\": \"" + node->nodeName() + "\"";
> +    else if (node->isTextNode())
> +        result += "\"value\": " + quoteAndEscapeNonPrintables(node->nodeValue());
> +
> +    result += ", \"ancestors\": \"";
> +    ContainerNode* ancestor = node->parentOrHostNode();
> +    while (ancestor) {
> +        result += ancestor->nodeName();
> +        ancestor = ancestor->parentOrHostNode();
> +        if (ancestor)
> +            result += ",";
> +    }
> +    return result + "\" }";
> +}

Is there a way we can push this into DumpRenderTree?  It's not great that we'll be shipping a function that's only used in DRT.

Also, I'm not sure if JSON is the correct format.  nodeName is usually not enough for this kind of purpose because they'll almost all end up #text.

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> (From update of attachment 92316 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92316&action=review
> 
> > Source/WebCore/dom/Position.cpp:1240
> > +static String nodeAsJSONString(Node* node)
> > +{
> > +    String result = "{ ";
> > +    // { name|value, ancestors }
> > +    if (node->isElementNode() || node->isDocumentNode())
> > +        result += "\"name\": \"" + node->nodeName() + "\"";
> > +    else if (node->isTextNode())
> > +        result += "\"value\": " + quoteAndEscapeNonPrintables(node->nodeValue());
> > +
> > +    result += ", \"ancestors\": \"";
> > +    ContainerNode* ancestor = node->parentOrHostNode();
> > +    while (ancestor) {
> > +        result += ancestor->nodeName();
> > +        ancestor = ancestor->parentOrHostNode();
> > +        if (ancestor)
> > +            result += ",";
> > +    }
> > +    return result + "\" }";
> > +}
> 
> Is there a way we can push this into DumpRenderTree?  It's not great that we'll be shipping a function that's only used in DRT.

Pushing this into DRT seems bad for 2 reasons:

1) we would need to reveal more API to query VisiblePosition from outside WebCore
2) the code would basically have to be duplicated across all of the ports.

As for shipping code only used in DRT, there are plenty of examples of that already. See layerTreeAsText or RenderTreeAsText.

> 
> Also, I'm not sure if JSON is the correct format.  nodeName is usually not enough for this kind of purpose because they'll almost all end up #text.

JSON is awesome, because it allows us to snapshot the information and pass it along as an opaque string. When receiving it in the test, you can either dump it as is, or use JSON.parse() to treat it as an object. The #text is handled very nicely, too -- you'll see that I actually store its value, not name.

[Alexey Proskuryakov]

As discussed at the contributor meeting, we should be putting code like this in a separate library that will be used by DRT/WRT. Would you perhaps be wiling to help with that effort?

[WebKit Review Bot]

Attachment 92316 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8571219

[Dimitri Glazkov (Google)]

(In reply to comment #6)
> As discussed at the contributor meeting, we should be putting code like this in a separate library that will be used by DRT/WRT. Would you perhaps be wiling to help with that effort?

Oh no, it's a matryoshka yak shave! As much as it pains me to say it, yes.

[Ryosuke Niwa]

On my second thought, this isn't even adequate for other elements. For example, the default paragraph separator is div so if we have two paragraphs, and we just see [div, 0], we wouldn't know whether it's the first div or the second div.

[Dimitri Glazkov (Google)]

(In reply to comment #9)
> On my second thought, this isn't even adequate for other elements. For example, the default paragraph separator is div so if we have two paragraphs, and we just see [div, 0], we wouldn't know whether it's the first div or the second div.

Yeah, discerning siblings is something I ran into right away trying to convert your readonly inputs tests. I was experimenting with adding a nodeIndex to the ancestors to alleviate this, but this is all now on hold, because I need to get the WebCoreTestSupport thingy running first.

[Dimitri Glazkov (Google)]

Will start with studying bug 42612 tomorrow.

[Alexey Proskuryakov]

Please consider not diverging too much from editing delegate style output, see e.g. <http://trac.webkit.org/browser/trunk/LayoutTests/editing/execCommand/switch-list-type-expected.txt>.

[Dimitri Glazkov (Google)]

(In reply to comment #12)
> Please consider not diverging too much from editing delegate style output, see e.g. <http://trac.webkit.org/browser/trunk/LayoutTests/editing/execCommand/switch-list-type-expected.txt>.

The beauty of the JSON object approach is that it doesn't produce any output. You can test it as finely or as coarsely as your test requires.

[Dimitri Glazkov (Google)]

Created attachment 92370 [details]
WIP: Selection test harness.

[Dimitri Glazkov (Google)]

Comment on attachment 92370 [details]
WIP: Selection test harness.

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

Here's something more than excited blabbering about wonders of JSON objects.

> LayoutTests/editing/selection-harness-unit-test.html:7
> +var sampleData = {

Here's how typical data would look when it comes out of currentSelection().

> LayoutTests/editing/selection.js:45
> +            return s + [ 'in', 'after', 'before' ][this.type] + ', offset: ' + this.offset;

This can be made to match EDITING DELEGATE output. Easily -- no need to rebuild WebKit :)

[Dimitri Glazkov (Google)]

Created attachment 92373 [details]
WIP: Added the new test, which was missing from previous patch.

[Dimitri Glazkov (Google)]

Comment on attachment 92373 [details]
WIP: Added the new test, which was missing from previous patch.

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

> LayoutTests/editing/selection/select-across-readonly-input.html:12
> +            return selection.start.anchor.isDescendantOf('INPUT') && selection.end.anchor.isDescendantOf('INPUT');

... and here's how you would use it.

> LayoutTests/editing/selection/select-across-readonly-input.html:36
> +            return selection.start.anchor.equals(selection.end.anchor);

.. or like this.

[Ryosuke Niwa]

(In reply to comment #10)
> (In reply to comment #9)
> > On my second thought, this isn't even adequate for other elements. For example, the default paragraph separator is div so if we have two paragraphs, and we just see [div, 0], we wouldn't know whether it's the first div or the second div.
> 
> Yeah, discerning siblings is something I ran into right away trying to convert your readonly inputs tests. I was experimenting with adding a nodeIndex to the ancestors to alleviate this, but this is all now on hold, because I need to get the WebCoreTestSupport thingy running first.

Sibling is one thing but we usually have to deal with a large DOM with lots of nested divs.  And I don't think element name is helpful in distinguish those either.

[Dimitri Glazkov (Google)]

(In reply to comment #18)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > On my second thought, this isn't even adequate for other elements. For example, the default paragraph separator is div so if we have two paragraphs, and we just see [div, 0], we wouldn't know whether it's the first div or the second div.
> > 
> > Yeah, discerning siblings is something I ran into right away trying to convert your readonly inputs tests. I was experimenting with adding a nodeIndex to the ancestors to alleviate this, but this is all now on hold, because I need to get the WebCoreTestSupport thingy running first.
> 
> Sibling is one thing but we usually have to deal with a large DOM with lots of nested divs.  And I don't think element name is helpful in distinguish those either.

Yes, but nodeIndex is.

[Dimitri Glazkov (Google)]

(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > On my second thought, this isn't even adequate for other elements. For example, the default paragraph separator is div so if we have two paragraphs, and we just see [div, 0], we wouldn't know whether it's the first div or the second div.
> > > 
> > > Yeah, discerning siblings is something I ran into right away trying to convert your readonly inputs tests. I was experimenting with adding a nodeIndex to the ancestors to alleviate this, but this is all now on hold, because I need to get the WebCoreTestSupport thingy running first.
> > 
> > Sibling is one thing but we usually have to deal with a large DOM with lots of nested divs.  And I don't think element name is helpful in distinguish those either.
> 
> Yes, but nodeIndex is.

In other words, instead of ancestors: "DIV,INPUT,BODY,HTML,#document", we would have ancestors: "DIV[0],INPUT[1],BODY[0],HTML[0],#document".

[Dimitri Glazkov (Google)]

This is no longer necessary.