[webkit-reviews] review granted: [Bug 268920] AX: Adopt non-blinking cursor API : [Attachment 469856] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 13 16:39:26 PST 2024

Tyler Wilcock <tyler_w at apple.com> has granted  review:
Bug 268920: AX: Adopt non-blinking cursor API

Attachment 469856: Patch


--- Comment #21 from Tyler Wilcock <tyler_w at apple.com> ---
Comment on attachment 469856
  --> https://bugs.webkit.org/attachment.cgi?id=469856

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

> Source/WebCore/testing/Internals.cpp:1140
> +    Document* document = contextDocument();


> Source/WebCore/testing/Internals.cpp:1905
> +ExceptionOr<bool> Internals::isCaretBlinkingSuspended(Document& document)
> +{
> +    if (!document.frame())
> +	   return Exception { ExceptionCode::InvalidAccessError };
> +
> +    return document.frame()->selection().isCaretBlinkingSuspended();
> +}

Can we change the implementation of Internals::isCaretBlinkingSuspended() to
call this method with `contextDocument()` so we aren't duplicating the logic?

> Source/WebCore/testing/Internals.idl:508
>      boolean isCaretBlinkingSuspended();
> +    boolean isCaretBlinkingSuspended(Document document);

Maybe we can add a comment explaining what document the parameter-less method
queries now that we have two methods with the same name?

> LayoutTests/accessibility/mac/prefers-non-blinking-cursor.html:15
> +let iframe = document.getElementById("iframe")

Can this iframe be const? Also it's missing a semi-colon at the end of the
statement, might be nice to consistent with all of our other statements which
end in semicolons.

More information about the webkit-reviews mailing list