[webkit-reviews] review granted: [Bug 220607] Elements in a table are incorrectly selected in JavaScript. : [Attachment 417909] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 220607: Elements in a table are incorrectly selected in JavaScript.
https://bugs.webkit.org/show_bug.cgi?id=220607

Attachment 417909: Patch

https://bugs.webkit.org/attachment.cgi?id=417909&action=review




--- Comment #6 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 417909
  --> https://bugs.webkit.org/attachment.cgi?id=417909
Patch

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

> LayoutTests/editing/selection/editable-table-cell-selection.html:13
> +	       if (!testRunner.runUIScript)

I don't see any (direct or indirect) uses of runUIScript in this test…can we
just remove this early return and make the test manually runnable?

> LayoutTests/editing/selection/editable-table-cell-selection.html:18
> +	       function select( element ) {

Nit - lots of extra spaces in function calls in runTest().

> LayoutTests/editing/selection/editable-table-cell-selection.html:21
> +		   range.setStartBefore( element );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:22
> +		   range.setEndAfter( element );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:27
> +		   selection.addRange( range );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:30
> +	       select( document.querySelector( 'thead tr' ) );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:37
> +	       const lastHeadCell = [ ...document.querySelectorAll( 'thead th'
) ].pop();

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:38
> +	       select( lastHeadCell );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:45
> +	       select( document.querySelector( 'tbody tr' ) );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:47
> +		   output += 'PASS: Correctly Selects row in tboyd';

Nit - tbody.

> LayoutTests/editing/selection/editable-table-cell-selection.html:52
> +	       const lastBodyCell = [ ...document.querySelectorAll( 'tbody td'
) ].pop();

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:53
> +	       select( lastBodyCell );

(Ditto)

> LayoutTests/editing/selection/editable-table-cell-selection.html:123
> +	   This test requires UIScriptController to run.

Hm…is this true? I don't see any UIScriptController bits in this test.