[Webkit-unassigned] [Bug 162139] REGRESSION(r194387): Crash on github.com in IntlDateTimeFormat::resolvedOptions in C locale

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=162139

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #8)
> Comment on attachment 289442 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289442&action=review
> 
> > LayoutTests/js/intl-invalid-locale-crash.html:10
> > +if (window.internals) {
> > +    window.internals.setUserPreferredLanguages(["a"]);
> 
> Please add a comment to indicate that you're doing this to ensure the script
> gets run with an invalid locale.

Ah, ok, I assumed the test name was enough.

> > Source/JavaScriptCore/runtime/IntlCollator.cpp:261
> > +        throwTypeError(&state, scope, ASCIILiteral("failed to initialize Collator"));
> 
> Wouldn't it be good to indicate why? "failed to initialize Collator due to
> invalid locale"?

Ok.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:976
> > +    for (size_t i = 0; languageList[i]; ++i) {
> > +        // Dot not propagate the C locale to WebCore.
> 
> "Do not"

Oops.

> And since you changed the line above anyway, I would change size_t to auto
> as well.

What would auto deduce for 0? int? I prefer to be explicit here, we want an unsigned type.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:-979
> > -    WebCore::languageDidChange();
> 
> What, why dropping this? This looks like an accident. Please don't commit
> without responding to this.

No, but I forgot to comment about this in the changelog, WebCore::overrideUserPreferredLanguages() already calls languageDidChange()), so we were calling all the observers twice.

> > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:491
> > +    // When using the C locale, en-US should be used as default.
> 
> I would copy/paste this entire block to add a test for POSIX locale as well.

Good point.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160922/34c69624/attachment.html>