| Summary: | REGRESSION(r259401): [GTK] Check surroundingRange is not null | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Diego Pino <dpino> | ||||||||
| Component: | New Bugs | Assignee: | Diego Pino <dpino> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | clopez, darin, pnormand, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=209723 https://bugs.webkit.org/show_bug.cgi?id=210149 |
||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 209723 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Diego Pino
2020-04-03 08:19:57 PDT
Created attachment 395377 [details]
Patch
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 Created attachment 395382 [details]
Patch
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. Created attachment 395383 [details]
Patch
Committed r259468: <https://trac.webkit.org/changeset/259468> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395383 [details]. |