[Webkit-unassigned] [Bug 232420] Selection extend() should trigger exception with no ranges

Fri Oct 29 14:31:21 PDT 2021

https://bugs.webkit.org/show_bug.cgi?id=232420

Chris Dumez <cdumez at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #442859|1                           |0
        is obsolete|                            |

--- Comment #14 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 442859
  --> https://bugs.webkit.org/attachment.cgi?id=442859
Patch

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

> LayoutTests/editing/execCommand/null_calc_primitive_value_for_css_property.html:16
> +    document.getSelection().addRange(range);

Wouldn't `document.getSelection().collapse(x, 0);` work?

> LayoutTests/editing/execCommand/transpose-backslash-with-euc.html:13
> +    getSelection().addRange(new Range());

May be better to call:
getSelection().collapse(backslashDivElement, 0);
before calling extend.

> LayoutTests/editing/inserting/insert-html-crash-02.html:16
> +    getSelection().addRange(new Range());

Similarly, would be better to call getSelection().collapse(document.body, 0) I think.

> LayoutTests/imported/w3c/ChangeLog:9
> +        contains no Range objects.

Please provide a link to the specification (https://w3c.github.io/selection-api/#dom-selection-extend) since you are aligning with the spec.

Also please mention in the change log the behavior of other browser engines. Matching the spec is good but only if other engines agree with the spec.

> LayoutTests/imported/w3c/web-platform-tests/selection/extend-exception.html:2
> +<title>Selection extend() test thrown exceptions</title>

Missing <html> tag.

> LayoutTests/imported/w3c/web-platform-tests/selection/extend-exception.html:4
> +<meta name=timeout content=long>

This is a trivial test, it doesn't need a long timeout.

> Source/WebCore/page/DOMSelection.cpp:342
> +    if (this->rangeCount() < 1)

is this-> really needed?

> Source/WebCore/page/DOMSelection.cpp:343
> +        return Exception { InvalidStateError, "The 'extend' method requires a Range object to be added to the Selection object before calling this method." };

I think I'd prefer a message like: "extend() cannot be called on an empty selection"

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211029/4053e9cc/attachment.htm>