Bug 209966 – REGRESSION(r259401): [GTK] Check surroundingRange is not null

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Diego Pino]

REGRESSION(r259401): [GTK] Check surroundingRange is not null

[Diego Pino]

Created attachment 395377 [details]
Patch

[Philippe Normand]

Comment on attachment 395377 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/glib/WebPageGLib.cpp:116
> +            if (surroundingRange != nullptr)

We usually don't do explicit checks like this. Can you remove the `!= nullptr` part?

> Source/WebKit/WebProcess/WebPage/glib/WebPageGLib.cpp:121
> +            if (surroundingRange != nullptr)

Ditto

[Diego Pino]

Created attachment 395382 [details]
Patch

[Darin Adler]

Comment on attachment 395382 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/glib/WebPageGLib.cpp:117
>              surroundingRange->setEnd(compositionRange->startPosition());
>              clonedRange->setStart(compositionRange->endPosition());
> -            postLayoutData.surroundingContext = plainText(*surroundingRange) + plainText(clonedRange);
> +            if (surroundingRange)
> +                postLayoutData.surroundingContext = plainText(*surroundingRange) + plainText(clonedRange);

This change isn’t needed. As you can see, surroundingRange is already used above and so it can’t be null by the time it gets here.

> Source/WebKit/WebProcess/WebPage/glib/WebPageGLib.cpp:122
> -            postLayoutData.surroundingContext = plainText(*surroundingRange);
> +            if (surroundingRange)
> +                postLayoutData.surroundingContext = plainText(*surroundingRange);

This change looks OK. Sorry for missing it. To preserve historical behavior, could instead write:

    postLayoutData.surroundingContext = surroundingRange ? plainText(*surroundingRange) : emptyString();

The old code set the string to empty string, rather than leaving it with its previous value.

[Diego Pino]

Created attachment 395383 [details]
Patch

[EWS]

Committed r259468: <https://trac.webkit.org/changeset/259468>

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

[Radar WebKit Bug Importer]

<rdar://problem/61263510>