[webkit-reviews] review granted: [Bug 16815] Crash with navigator.plugins and navigator.mimeTypes after plugins.refresh : [Attachment 19587] updated patch rebased against trunk including the latest suggestion and idl fixes

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 7 12:22:27 PST 2008


Darin Adler <darin at apple.com> has granted Simon Hausmann <hausmann at kde.org>'s
request for review:
Bug 16815: Crash with navigator.plugins and navigator.mimeTypes after
plugins.refresh
http://bugs.webkit.org/show_bug.cgi?id=16815

Attachment 19587: updated patch rebased against trunk including the latest
suggestion and idl fixes
http://bugs.webkit.org/attachment.cgi?id=19587&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'm annoyed by the use of Mime in this new code and MIME in all existing
WebCore code. I would have preferred to stick with MIME.

 1598	      String pluginName;
 1599	      if (m_frame->page())
 1600		  pluginName =
m_frame->page()->pluginData()->pluginNameForMimeType(mimeType);
15991601	 if (!pluginName.isEmpty() && !pluginName.contains("QuickTime",
false)) 
16001602	     return true;

This could have been done with a nested if; obviously the "return true" is not
needed if the page is 0.

In fact, we could have just added the "page" check to the if above this.

 67	for (unsigned i = 0; i < mimes.size(); ++i)
 68	    if (mimes[i] == mime)
 69		return MimeType::create(m_pluginData.get(), i).get();

These should have braces.

r=me

Is there a way to make a regression test for this? We normally require them.


More information about the webkit-reviews mailing list