[webkit-reviews] review granted: [Bug 177991] Split StaticPasteboard::writeString into writeString and writeStringInCustomData : [Attachment 322986] Cleanup

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 5 23:22:28 PDT 2017


Wenson Hsieh <wenson_hsieh at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 177991: Split StaticPasteboard::writeString into writeString and
writeStringInCustomData
https://bugs.webkit.org/show_bug.cgi?id=177991

Attachment 322986: Cleanup

https://bugs.webkit.org/attachment.cgi?id=322986&action=review




--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 322986
  --> https://bugs.webkit.org/attachment.cgi?id=322986
Cleanup

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

> Source/WebCore/dom/DataTransfer.cpp:162
> +	   m_itemList->didSetStringData(type);

Is the change from normalizedType => type intended? This would make it so that
if you setData('text', 'foo') after the item list has been ensured, the
DataTransferItemList will contain an item of type 'text', rather than
'text/plain'.

> Source/WebCore/platform/StaticPasteboard.cpp:63
> +    updateTypes(m_types, type, !m_platformData.set(type, value).isNewEntry
|| m_customData.contains(type));

I think this code is a little hard to follow — I imagined this would look
something like

writeString:

    moveToEndOfTypes(type);
    m_platformData.set(type, value);

...and writeStringInCustomData:

    moveToEndOfTypes(type);
    m_customData.set(type, value);

...and the private helper method moveToEndOfTypes would just do:

    if (m_types.contains(types))
	m_types.removeFirst(type);
    m_types.append(type);

(or if we want to avoid the linear search through all types)

    if (m_customData.contains(type) || m_platformData.contains(type))
	m_types.removeFirst(type);
    m_types.append(type);

...but I don't think it's that big a deal. Up to you!


More information about the webkit-reviews mailing list