Restore charset title fallback

RESOLVED FIXED in mozilla19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: neil, Assigned: neil)

Tracking

unspecified
mozilla19
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Assignee

Description

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

Comment 1

7 years ago
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #672519 - Flags: review?(smontagu)
Neil, I hope you don't mind, but I thought this function could use some more work than you gave it :)
Attachment #672525 - Flags: review?(smontagu)
Assignee

Comment 3

7 years ago
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.
Assignee

Comment 4

7 years ago
Compiles, but not tried to link or test ;-)
Attachment #672599 - Flags: feedback?(Ms2ger)
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 :)
Attachment #672599 - Flags: feedback?(Ms2ger) → feedback+
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.
Attachment #672519 - Flags: review?(smontagu) → review+
Attachment #672525 - Flags: review?(smontagu)
https://hg.mozilla.org/mozilla-central/rev/7af92c476876
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.