802820 - Restore charset title fallback

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[[e-mail]]

Bug 777292 inadvertently made a missing charset title failure throw an exception and cause the relevant charset menu not to be built.

Comment 1

[[e-mail]]

Attachment: Possible patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Another possible patch

Neil, I hope you don't mind, but I thought this function could use some more work than you gave it :)

Comment 3

[[e-mail]]

a) I specifically asked smontagu on IRC which approach he wanted
b) the whole file needs a rewrite, nsMenuItem should be a struct with two non-auto strings that gets copied by value instead of passing around pointers and hoping that you didn't leak.

Comment 4

[[e-mail]]

Attachment: Alternative approach

Compiles, but not tried to link or test ;-)

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 672599 [details] [diff] [review]
Alternative approach

Review of attachment 672599 [details] [diff] [review]:
-----------------------------------------------------------------

That was rather more than I expected ;)

f=me as long as I can deny having ever seen this code.

::: toolkit/components/intl/nsCharsetMenu.cpp
@@ +1063,3 @@
>  
>    done:
> +    ;

All the failure cases can just return, because they would only set mAutoDetectInitialized to false, which it already is.

@@ -1110,5 @@
>    res = AddMenuItemArrayToContainer(container, &moreMenu, NULL);
>    if (NS_FAILED(res)) goto done;
>  
>  done:
> -  // free the elements in the VoidArray

Heh.

@@ +1098,1 @@
>    return res;

Bonus points for getting rid of all the gotos here :)

Comment 6

[Simon Montagu :smontagu]

Comment on attachment 672519 [details] [diff] [review]
Possible patch

Review of attachment 672519 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. The other patches are way out of scope for this bug but feel free to open new bugs for refactoring.

Comment 7

[[e-mail]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7af92c476876

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7af92c476876