[Webkit-unassigned] [Bug 228122] Expand URL class query parameter functions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=228122

--- Comment #13 from Risul Islam <risul_islam at apple.com> ---
Comment on attachment 434642
  --> https://bugs.webkit.org/attachment.cgi?id=434642
Patch

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

>> Source/WTF/wtf/URL.cpp:1224
>> +    auto thisPathLength = m_string[m_pathEnd - 1] == '/' ? m_pathEnd - 1 : m_pathEnd;
> 
> I don't think this does what you think it does.  You have a test with "http://www.webkit.org?" but when parsed and canonicalized it becomes "http://www.webkit.org/?". Add a test with a zero length path, like custom-scheme://host?a=b and http://example.com/?a=b

Sorry for the confusion. I am changing thisPathLength to thisLengthOfURLIgnoringQueryAndFragments.

>> Source/WTF/wtf/URL.cpp:1230
>> +bool URL::containsSameQueryParameters(const URL& otherURL) const
> 
> Do we want http://example.com/ and http://example.com/? to be considered to have the same query parameters?  Add that test.

Yes, we want to consider them equal. Adding test.

>> Source/WTF/wtf/URL.cpp:1254
>> +            queryWithoutRemovalKeys = makeString(queryWithoutRemovalKeys, singleQueryParameter.key, '=', singleQueryParameter.value, '&');
> 
> This. should actually be a StringBuilder instead.  It's more efficient than this, which is currently an O(n^2) algorithm because of all the recopying of the current String.

Need a little bit more clarification.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210730/8bbf0ab9/attachment.htm>