[webkit-reviews] review granted: [Bug 209966] REGRESSION(r259401): [GTK] Check surroundingRange is not null : [Attachment 395382] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 3 09:04:39 PDT 2020


Darin Adler <darin at apple.com> has granted Diego Pino <dpino at igalia.com>'s
request for review:
Bug 209966: REGRESSION(r259401): [GTK] Check surroundingRange is not null
https://bugs.webkit.org/show_bug.cgi?id=209966

Attachment 395382: Patch

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




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 395382
  --> https://bugs.webkit.org/attachment.cgi?id=395382
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.


More information about the webkit-reviews mailing list