Bug 257414

Summary: Remove stripLeadingAndTrailingHTMLSpaces()
Product: WebKit Reporter: Anne van Kesteren <annevk>
Component: Web Template FrameworkAssignee: Anne van Kesteren <annevk>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 257438    
Bug Blocks: 255467    

Anne van Kesteren
Reported 2023-05-27 01:34:53 PDT
Now that we have trim() this abstraction isn't really needed anymore.
Attachments
Radar WebKit Bug Importer
Comment 1 2023-05-27 01:35:16 PDT
Anne van Kesteren
Comment 2 2023-05-27 02:11:39 PDT
Unfortunately this is not a straightforward fix due to the existence of NSString. Is there a way to convert those to String or StringView so trim() can then be used? Search for HTMLSpace in https://github.com/WebKit/WebKit/blob/main/Source/WebCore/editing/cocoa/HTMLConverter.mm to see some instances of the problem. The other thing I noticed is that we're invoking this a lot before passing the string to the URL parser (often via completeURL()). However, the URL parser will trim all C0 control or space code points as per https://url.spec.whatwg.org/#concept-basic-url-parser (and as far as I can tell our implementation matches that). This means that we're doing the work twice. Perhaps someone can confirm I'm correct here and then I could create an initial PR that just removes these callers as they are redundant with the URL parser.
Alex Christensen
Comment 3 2023-05-27 08:37:11 PDT
I'm pretty sure the calls to stripLeadingAndTrailingHTMLSpaces in HTMLConverter.mm are implicitly calling the WTF::String constructor from an NSString *. I'm pretty sure trimming space before passing to the URL parser is redundant work.
Anne van Kesteren
Comment 4 2023-05-30 00:46:37 PDT
EWS
Comment 5 2023-05-30 22:57:05 PDT
Committed 264713@main (ea527fa9c22d): <https://commits.webkit.org/264713@main> Reviewed commits have been landed. Closing PR #14467 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.