[Webkit-unassigned] [Bug 36482] Regression(r55766): [Chromium] chromiumDataObject.hasData() reports true for an empty clipboard

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Roland Steiner <rolandsteiner at chromium.org>  2010-03-23 20:53:12 PST ---
(In reply to comment #3)
> (From update of attachment 51403 [details])
> > +        No new tests. (simple code change)
> 
> Is it possible to write a layout test for this?  Would be nice to make sure we
> don't regress in the future.

This is hit in chrome/src/webkit/glue/webdropdata.cc, during the conversion
from WebDropData to WebDragData. This doesn't seem to be used in the layout
test shell.
So I'm not sure there is a easy way to test this (except adding a spurious copy
operation somewhere), and adding an interface seemed overkill for such a small
change.

> > +void ChromiumDataObject::setURL(const KURL& newURL)
> > +{
> > +    ASSERT(newURL.isValid() || newURL.isEmpty());
> 
> Do we really want to assert here?  Can't javascript try to set an invalid URL? 
> We shouldn't assert in that case.

This should not be exposed to JavaScript directly - for JS, sanitizing is done
in ClipboardChromium::setData().

Conceivably, there may be an invalid URL passed-in on the clipboard from an
external process, but again I think it'd be better if we sanitized this before
accepting it. Hence the ASSERT.

However, I don't feel strongly about this and could be convinced that we want
to accept invalid URLs in the clipboard data, and remove the ASSERT.

> > +    uriList.clear();
> > +    if (newURL.isValid()) {
> 
> What about the constructor?  It doesn't go through this code path.

There is only a default and a copy constructor. For the first there is no
issue, and for the second it's assumed the data in the copied
ChromiumDataObject is already legal.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.