Do the Qt equivalent of https://bugs.webkit.org/show_bug.cgi?id=31352 and http://trac.webkit.org/changeset/51104
Created attachment 113300 [details] patch media/video-document-types.html (when removed from the QT skipped list) runs fine now with the latest code base. But logically, we still need to cancel the main resource load, if the data is handled by MediaDocument. Webkit2 as well as all other ports (GTK/Chromium etc) do that. In my patch I didn't un-skip the media/video-document-types.html because the media folder itself is in skipped list.
(In reply to comment #1) > Created an attachment (id=113300) [details] > patch > > media/video-document-types.html (when removed from the QT skipped list) runs fine now with the latest code base. > > But logically, we still need to cancel the main resource load, if the data is handled by MediaDocument. > Webkit2 as well as all other ports (GTK/Chromium etc) do that. > > In my patch I didn't un-skip the media/video-document-types.html because the media folder itself is in > skipped list. LGTM. I was going to fix our WebKit1 shouldFallBack implementation, so thanks! :) I'm not 100% sure about the commitLoad part but I did a quick check and other ports are handling it like that, as you said. Isn't there any other tests we can un-skip after this? By the way, can't we un-skip the media folder and this test and then just skip everything else in there? Thanks again!
Comment on attachment 113300 [details] patch r=me
Comment on attachment 113300 [details] patch Clearing flags on attachment: 113300 Committed r99071: <http://trac.webkit.org/changeset/99071>
All reviewed patches have been landed. Closing bug.
Reopen, because it broke 4 layout tests on the Qt bot: http://build.webkit.org/results/Qt%20Linux%20Release/r99073%20%2839226%29/results.html Could you check it please?
(In reply to comment #6) > Reopen, because it broke 4 layout tests on the Qt bot: > http://build.webkit.org/results/Qt%20Linux%20Release/r99073%20%2839226%29/results.html > > Could you check it please? I spent some time debugging this issue and found the root cause. The problem is not with my patch, but the enum that we have borrowed from mac (code from FrameLoaderClientQt.cpp:930) // This was copied from file "WebKit/Source/WebKit/mac/Misc/WebKitErrors.h". enum { WebKitErrorCannotShowMIMEType = 100, WebKitErrorCannotShowURL = 101, WebKitErrorFrameLoadInterruptedByPolicyChange = 102, WebKitErrorCannotUseRestrictedPort = 103, WebKitErrorCannotFindPlugIn = 200, WebKitErrorCannotLoadPlugIn = 201, WebKitErrorJavaUnavailable = 202, WebKitErrorPluginWillHandleLoad = 203 }; shouldFallBack() tells the DOM if it should attempt to render the next nested <object> if its parent fails to load. Now when a url fails to load, QNetworkReplyHandler sets the QNetworkReply::NetworkError ( reply->error() any one of all possible error conditions found during the processing of the request) and converts that to ResourceError object setting appropriate fields like domain, error code, description etc. The QNetworkReply::NetworkError constant values clash with values selected for our enum (listed above). ie: QNetworkReply::NetworkError constant values : Constant Value ===================================== ProxyConnectionRefusedError 101 ProxyConnectionClosedError 102 ProxyNotFoundError 103 ProxyTimeoutError 104 ProxyAuthenticationRequiredError105 ContentAccessDenied 201 ContentOperationNotPermittedError202 ContentNotFoundError 203 AuthenticationRequiredError 204 ContentReSendError 205 ProtocolUnknownError 301 ProtocolInvalidOperationError 302 UnknownNetworkError 99 UnknownProxyError 199 UnknownContentError 299 ProtocolFailure 399 In the failing test case we get error.errorCode() as 203, which with respect to network error is 'ContentNotFoundError' and with respect to our enum is 'WebKitErrorPluginWillHandleLoad'. Changing the enum values for WebKitError's to some other constant values like WebKitErrorPluginWillHandleLoad = 2003, Then the failing test cases work fine. I think to track changes better we should file a separate bug for resolving this enum issue and close the current reopened issue. Let me know if you have any comments ???? also let me know if I can go ahead and file a separate bug and a patch for that.
Thanks for debugging. I agree with you, please file a separate bug.
I added the failing tests to the skipped list to make buildbot happier: http://trac.webkit.org/changeset/99168 . Please unskip them with the fix.
(In reply to comment #9) > I added the failing tests to the skipped list to make buildbot happier: > http://trac.webkit.org/changeset/99168 . Please unskip them with the fix. Thanks Csaba Osztrogonac. Done. https://bugs.webkit.org/show_bug.cgi?id=71554 (In reply to comment #2) > By the way, can't we un-skip the media folder and this test and then just skip > everything else in there? I actually tried that with my local QT build. Many of the skipped test cases for media do work fine. Atleast on my local qt linux build. I wasn't sure of the build-bot environment/configuration, hence didn't suggest this :). With support of gstreamer coming in and extension map now supporting local media files, I think your suggestion makes absolute sense.
(In reply to comment #9) > I added the failing tests to the skipped list to make buildbot happier: > http://trac.webkit.org/changeset/99168 . Please unskip them with the fix. BTW . I feel this issue can now be closed.
Hi Csaba , bug 71554 is now resolved fixed. Request you to close this bug as well. Let me know if you need something from my side.
Thanks for the fix.