[Webkit-unassigned] [Bug 228122] Add functionalities for parsing URL query string

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 3 11:11:02 PDT 2021


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

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

>>> Source/WTF/wtf/KeyValuePair.h:67
>>> +    }
>> I’d like to see this be done as a free function, after the class definition:
>>     template<typename KeyType, typename ValueType> constexpr bool operator=(const KeyValuePair<KeyType, ValueType>& a, const KeyValuePair<KeyType, ValueType>& b)
>>     {
>>         return a.key == b.key && a.value == b.value;
>>     }
>> But also, this change is no longer needed in this patch. We aren’t using a KeyValuePair as a hash table key. Can we either not do this at all at this time, or do it in a separate patch?
> Thank you Darin for your valuable comments. All of your suggestions are great learning for me.
> I have removed the hash related codes. But removing this == operator overloading causes error in our test functions, specifically in EXPECT_EQ(differingQueryParameters(url1, url2), resultVector);
> Any workaround suggestion?

Making the == operator a free function was a great idea. We do need this function for the purpose of testability of differingQueryParameters(url, url) function. I am submitting the new patch with it. If we find any workaround, we may discard the == operator overloaded function quickly.

>>> Source/WTF/wtf/URL.cpp:1240
>>> +    return url.string()[url.pathEnd() - 1] == '/' ? url.pathEnd() - 1 : url.pathEnd();
>> I don’t understand why this "/" removal is needed. Which test case falls if we remove this logic?
> Thank you and Alex for this suggestion. Canonicalization removes the requirement of this log. Discarding it.

Thank you and Alex for this suggestion. Canonicalization removes the requirement of this *logic. Discarded it.

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/20210803/a70c914a/attachment.htm>

More information about the webkit-unassigned mailing list