Bug 257867

<rdar://problem/110488738>

Created attachment 466642 [details]
Patch

Created attachment 466644 [details]
Patch

Created attachment 466652 [details]
Patch

Created attachment 466653 [details]
Patch

Created attachment 466654 [details]
Patch

Comment on attachment 466654 [details]
Patch

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

> Source/WebKit/UIProcess/mac/WebViewImpl.mm:5111
> +            auto it = annotations.find(key);
> +            if (it == annotations.end())
> +                it = annotations.add(key, Vector<CharacterRange> { }).iterator;

I think HashMap::ensure does this (find a value, initializing it if necessary). Might express the intent a bit better (if it works here).

> Source/WebKit/UIProcess/mac/WebViewImpl.mm:5150
>              underlines = compositionUnderlines(string);
> +        annotations = compositionAnnotations(string);

The indentation is different between these two lines. Not sure which is right, though.

> Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:669
> +            auto it = annotations.find(name);
> +            if (it == annotations.end())
> +                it = annotations.add(name, Vector<WebCore::CharacterRange> { }).iterator;

Another place where HashMap::ensure might work?

I didn't know the function of HashMap::ensure, but now that I do, I'll switch to using that. Thanks!

As for the indentation difference, the first line is the single-line else clause if HAVE(REDESIGNED_TEXT_CURSOR) is true, otherwise, it's just part of the same scope as the annotations line I added. I think the thought was it's safer to leave it indented to convey that it's part of an else block, but that was not code I wrote.

> As for the indentation difference, the first line is the single-line else
> clause if HAVE(REDESIGNED_TEXT_CURSOR) is true, otherwise, it's just part of
> the same scope as the annotations line I added. I think the thought was it's
> safer to leave it indented to convey that it's part of an else block, but
> that was not code I wrote.
Got it, sounds good, thanks!

Committed 265055@main (ef93c1b412c0): <https://commits.webkit.org/265055@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466654 [details].

Test gardening commit 279418@main (190ac0f18d5d): <https://commits.webkit.org/279418@main>

Reviewed commits have been landed. Closing PR #29202 and removing active labels.