RESOLVED FIXED 61578
HTMLVideoElement::currentSrc() should return a KURL
https://bugs.webkit.org/show_bug.cgi?id=61578
Summary HTMLVideoElement::currentSrc() should return a KURL
WebKit Review Bot
Reported 2011-05-26 16:29:46 PDT
HTMLVideoElement::currentSrc() should return a KURL Requested by abarth on #webkit.
Attachments
Patch (8.18 KB, patch)
2011-05-27 00:43 PDT, Adam Barth
no flags
Patch for landing (9.89 KB, patch)
2011-05-27 10:51 PDT, Adam Barth
no flags
Adam Barth
Comment 1 2011-05-27 00:43:52 PDT
Alexis Menard (darktears)
Comment 2 2011-05-27 05:11:44 PDT
Comment on attachment 95132 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95132&action=review > Source/WebCore/html/canvas/CanvasRenderingContext.cpp:75 > + // HTMLMediaElement.currentSrc DOM API would leak redirect detinations! Typo here. > Source/WebCore/rendering/HitTestResult.cpp:315 > + return mediaElt->currentSrc(); I believe this is the same code because currentSrc has already his baseURL set.
Eric Seidel (no email)
Comment 3 2011-05-27 07:00:12 PDT
Comment on attachment 95132 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95132&action=review > Source/WebCore/html/HTMLMediaElement.cpp:678 > + LOG(Media, "HTMLMediaElement::loadResource(%s, %s)", urlForLogging(initialURL).utf8().data(), contentType.raw().utf8().data()); Ha. Seems urlForLogging should return a char*... >> Source/WebCore/rendering/HitTestResult.cpp:315 >> + return mediaElt->currentSrc(); > > I believe this is the same code because currentSrc has already his baseURL set. Is this tested anywhere? How do I as a reviewer know it's already absolute. :)
Adam Barth
Comment 4 2011-05-27 10:45:34 PDT
(In reply to comment #3) > (From update of attachment 95132 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95132&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:678 > > + LOG(Media, "HTMLMediaElement::loadResource(%s, %s)", urlForLogging(initialURL).utf8().data(), contentType.raw().utf8().data()); > > Ha. Seems urlForLogging should return a char*... Very true. > >> Source/WebCore/rendering/HitTestResult.cpp:315 > >> + return mediaElt->currentSrc(); > > > > I believe this is the same code because currentSrc has already his baseURL set. > > Is this tested anywhere? How do I as a reviewer know it's already absolute. :) I'm not sure whether this is tested, but AFAIK the KURL type only holds absolute URLs, so the C++ type system (such as it is) can tell you that. :)
Adam Barth
Comment 5 2011-05-27 10:51:56 PDT
Created attachment 95190 [details] Patch for landing
WebKit Commit Bot
Comment 6 2011-05-27 12:53:59 PDT
Comment on attachment 95190 [details] Patch for landing Clearing flags on attachment: 95190 Committed r87539: <http://trac.webkit.org/changeset/87539>
WebKit Commit Bot
Comment 7 2011-05-27 12:54:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.