Bug 187236 – [GTK] /webkit/WebKitWebView/mouse-target subtest fails

• Component: Tools / Tests
• Status: NEW

[Rob Buis]

/webkit/WebKitWebView/mouse-target from TestUIClient run FAILs.

[Rob Buis]

Created attachment 344057 [details]
Patch

[Daniel Bates]

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).

[Daniel Bates]

(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>

[Michael Catanzaro]

Comment on attachment 344057 [details]
Patch

Thanks for caring for the API tests, Rob! These are important and their current condition is embarrassing.

[Rob Buis]

Created attachment 344118 [details]
Patch

[Rob Buis]

(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.

[Daniel Bates]

(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?

[Rob Buis]

(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?

[Daniel Bates]

(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).

[Michael Catanzaro]

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.

[Rob Buis]

(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.