/webkit/WebKitWebView/mouse-target from TestUIClient run FAILs.
Created attachment 344057 [details] Patch
Comment on attachment 344057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344057&action=review > Tools/ChangeLog:9 > + video URL is not exposed this will fail. So allow local file access for the video Can you please elaborate on what you mean by "the local video URL is not exposed"? > Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:682 > + WebKitSettings* settings = webkit_web_view_get_settings(test->m_webView); > + webkit_settings_set_allow_universal_access_from_file_urls(settings, TRUE); Can you please explain why we need to universal access? I mean, this test only loads subresources with file URLs. (The markup is substituted for the request with URL file://// and movie.org is computed relative to it, file:///movie.ogg).
(In reply to Rob Buis from comment #0) > /webkit/WebKitWebView/mouse-target from TestUIClient run FAILs. For completeness, here is the error from the GTK Linux 64-bit Debug Tests bot that you are referring to: [[ /webkit/WebKitWebView/mouse-target: FAIL ERROR:/home/slave/webkitgtk/gtk-linux-64-debug/build/Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:734:void testWebViewMouseTarget(UIClientTest*, gconstpointer): assertion failed: (webkit_hit_test_result_context_is_media(hitTestResult)) ]] <https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Tests%29/builds/3307/steps/API%20tests/logs/stdio>
Comment on attachment 344057 [details] Patch Thanks for caring for the API tests, Rob! These are important and their current condition is embarrassing.
Created attachment 344118 [details] Patch
(In reply to Daniel Bates from comment #2) > Comment on attachment 344057 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=344057&action=review > > > Tools/ChangeLog:9 > > + video URL is not exposed this will fail. So allow local file access for the video > > Can you please elaborate on what you mean by "the local video URL is not > exposed"? > > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:682 > > + WebKitSettings* settings = webkit_web_view_get_settings(test->m_webView); > > + webkit_settings_set_allow_universal_access_from_file_urls(settings, TRUE); > > Can you please explain why we need to universal access? I mean, this test > only loads subresources with file URLs. (The markup is substituted for the > request with URL file://// and movie.org is computed relative to it, > file:///movie.ogg). Sorry my previous ChangeLog entry was a bit terse, I tried to phrase it better this time. Let me know if it needs even more rephrasing or you think this is the wrong approach.
(In reply to Rob Buis from comment #6) > (In reply to Daniel Bates from comment #2) > > Comment on attachment 344057 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=344057&action=review > > > > > Tools/ChangeLog:9 > > > + video URL is not exposed this will fail. So allow local file access for the video > > > > Can you please elaborate on what you mean by "the local video URL is not > > exposed"? > > > > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:682 > > > + WebKitSettings* settings = webkit_web_view_get_settings(test->m_webView); > > > + webkit_settings_set_allow_universal_access_from_file_urls(settings, TRUE); > > > > Can you please explain why we need to universal access? I mean, this test > > only loads subresources with file URLs. (The markup is substituted for the > > request with URL file://// and movie.org is computed relative to it, > > file:///movie.ogg). > > Sorry my previous ChangeLog entry was a bit terse, I tried to phrase it > better this time. Let me know if it needs even more rephrasing or you think > this is the wrong approach. The revised ChangeLog does not explain why a local file cannot load a local file as a subresource. Can you please explain this?
(In reply to Daniel Bates from comment #7) > (In reply to Rob Buis from comment #6) > > (In reply to Daniel Bates from comment #2) > > > Comment on attachment 344057 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=344057&action=review > > > > > > > Tools/ChangeLog:9 > > > > + video URL is not exposed this will fail. So allow local file access for the video > > > > > > Can you please elaborate on what you mean by "the local video URL is not > > > exposed"? > > > > > > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:682 > > > > + WebKitSettings* settings = webkit_web_view_get_settings(test->m_webView); > > > > + webkit_settings_set_allow_universal_access_from_file_urls(settings, TRUE); > > > > > > Can you please explain why we need to universal access? I mean, this test > > > only loads subresources with file URLs. (The markup is substituted for the > > > request with URL file://// and movie.org is computed relative to it, > > > file:///movie.ogg). > > > > Sorry my previous ChangeLog entry was a bit terse, I tried to phrase it > > better this time. Let me know if it needs even more rephrasing or you think > > this is the wrong approach. > > The revised ChangeLog does not explain why a local file cannot load a local > file as a subresource. Can you please explain this? "local file cannot load a local file as a subresource"? What file do you mean?
(In reply to Rob Buis from comment #8) > (In reply to Daniel Bates from comment #7) > > (In reply to Rob Buis from comment #6) > > > (In reply to Daniel Bates from comment #2) > > > > Comment on attachment 344057 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=344057&action=review > > > > > > > > > Tools/ChangeLog:9 > > > > > + video URL is not exposed this will fail. So allow local file access for the video > > > > > > > > Can you please elaborate on what you mean by "the local video URL is not > > > > exposed"? > > > > > > > > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:682 > > > > > + WebKitSettings* settings = webkit_web_view_get_settings(test->m_webView); > > > > > + webkit_settings_set_allow_universal_access_from_file_urls(settings, TRUE); > > > > > > > > Can you please explain why we need to universal access? I mean, this test > > > > only loads subresources with file URLs. (The markup is substituted for the > > > > request with URL file://// and movie.org is computed relative to it, > > > > file:///movie.ogg). > > > > > > Sorry my previous ChangeLog entry was a bit terse, I tried to phrase it > > > better this time. Let me know if it needs even more rephrasing or you think > > > this is the wrong approach. > > > > The revised ChangeLog does not explain why a local file cannot load a local > > file as a subresource. Can you please explain this? > > "local file cannot load a local file as a subresource"? What file do you > mean? The unit test code is loading the markup as substitute data for a file URL (i.e. we mimic a local file). This markup contains a <video> That loads file:///movie.ogg (a local file).
By default, file URIs do not have permission to load other file URIs. That's controlled by the property WebKitSettings::allow-file-access-from-file-urls. It might be sufficient to use webkit_settings_set_allow_file_access_from_file_urls() instead of the more heavyweight webkit_settings_set_allow_universal_access_from_file_urls() (which additionally allows access to e.g. localstorage). If so, that would be preferable.
(In reply to Michael Catanzaro from comment #10) > By default, file URIs do not have permission to load other file URIs. That's > controlled by the property WebKitSettings::allow-file-access-from-file-urls. > > It might be sufficient to use > webkit_settings_set_allow_file_access_from_file_urls() instead of the more > heavyweight webkit_settings_set_allow_universal_access_from_file_urls() > (which additionally allows access to e.g. localstorage). If so, that would > be preferable. I tried webkit_settings_set_allow_file_access_from_file_urls, unfortunately that is not enough to make the test PASS.