Bug 188111 – REGRESSION(r263094): [GTK][WPE] API test /webkit/WebKitWebContext/languages is failing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

API test /webkit/WebKitWebContext/languages always fails:

/webkit/WebKitWebContext/languages:                                 FAIL
ERROR:/home/slave/webkitgtk/gtk-linux-64-release/build/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:476:void testWebContextLanguages(WebViewTest*, gconstpointer): assertion failed: (!javascriptResult)

[Michael Catanzaro]

Nowadays it fails even earlier:

ERROR:../../Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:359:void testWebContextLanguages(WebViewTest*, gconstpointer): assertion failed (mainResourceDataSize == strlen(expectedLanguages)): (5 == 25)

Problem is the language is set to "en-US" rather than the expected "en,ES-es;q=0.90,dE;q=0.80".

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #1)
> Nowadays it fails even earlier:
> 
> ERROR:../../Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:
> 359:void testWebContextLanguages(WebViewTest*, gconstpointer): assertion
> failed (mainResourceDataSize == strlen(expectedLanguages)): (5 == 25)
> 
> Problem is the language is set to "en-US" rather than the expected
> "en,ES-es;q=0.90,dE;q=0.80".

This one is fixed by bug #222347

[Carlos Garcia Campos]

Created attachment 421392 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 421392 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421392&action=review

I was a bit confused how we wound up with a bug from 2018 fixing a regression from 2020. I see you just repurposed this bug to fix the new problem. OK.

> Source/WebKit/ChangeLog:9
> +        are no loner sending the new overrides to the web process. Instead of calling overrideUserPreferredLanguages()

no longer

[Michael Catanzaro]

Hi Chris, could you review this one please?

[Carlos Garcia Campos]

I didn't know this bug was so old.

[Michael Catanzaro]

Comment on attachment 421392 [details]
Patch

Anyway it looks good to me, but requires owner approval (ideally from Chris).

[Chris Dumez]

Comment on attachment 421392 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421392&action=review

> Source/WebKit/UIProcess/WebProcessPool.cpp:-308
> -    addLanguageChangeObserver(this, languageChanged);

I don't understand why we are getting rid of the language change observer in the UIProcess. Seems like we would want to know and notify the WebProcesses if the user changed the language, no?

[Carlos Garcia Campos]

(In reply to Chris Dumez from comment #9)
> Comment on attachment 421392 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=421392&action=review
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:-308
> > -    addLanguageChangeObserver(this, languageChanged);
> 
> I don't understand why we are getting rid of the language change observer in
> the UIProcess. Seems like we would want to know and notify the WebProcesses
> if the user changed the language, no?

Because after this patch overrideUserPreferredLanguages() is no longer called from the UI process. Right now is only called from the GLib API, unless I'm missing something.

[Carlos Garcia Campos]

Chris, are you ok with this then?

[Chris Dumez]

(In reply to Carlos Garcia Campos from comment #11)
> Chris, are you ok with this then?

I will look into this today. If this is not used on macOS then how do we detect and apply language change on macOS? macOS definitely has to do it in the UIProcess (and not WebProcess) due to sandboxing reasons). Seems like either macOS uses this or it doesn't and we may have a bug to fix (or there is another mechanism that macOS uses and that I am missing).

[Chris Dumez]

Comment on attachment 421392 [details]
Patch

LGTM. The WebProcess is able to listen to change notification on macOS.

[EWS]

Committed r273735: <https://commits.webkit.org/r273735>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421392 [details].