[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