516387 - nsPromptService dialogs should not have '&' shortcut in labels

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I get the unresponsive script warning on fan590.com on my n810 with swap enabled:)  This script has changed since the last time I saw it and there are some issues with it:
1) there is what appears to be a Yes/No slider like the preferences, but it is unclear, all the same color and says No
2) there is "&Don't ask me again".  Not sure what the & is about, I suspect it is for making the D a alt+ in windows.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

bug 514208 will address the theme issues. I'll turn this bug into dealing with the &

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch uses the code to strip & characters from any place that a users can pass a string for a label.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 400863 [details] [diff] [review]
patch

Fabrice - can you look this over too?

Comment 4

[Fabrice Desré]

Comment on attachment 400863 [details] [diff] [review]
patch

>diff --git a/chrome/content/prompt/promptPassword.xul b/chrome/content/prompt/promptPassword.xul  
>   <hbox id="prompt-password-checkbox-box" collapsed="true" align="center">
>     <checkbox class="toggle-dark" id="prompt-password-checkbox"/>
>-    <description id="prompt-password-checkbox-msg"/>
>+    <label id="prompt-password-checkbox-msg"/>
>   </hbox>
>   <hbox pack="center" id="prompt-password-button-box">

We should keep a <description> here since a <label> will not wrap if the text is too long and we probably don't want to crop it.

This means that :
- setLabelForNode() needs to check if aNode is a description to add a text child node.
- we also should make sure that other checkbox messages are descriptions, which is not the case currently.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 2

This patch addresses Fabrice's comments. The <label>s have been changed to <description>s and setLabelForNode checks for <label>s and <description>s, handling each a little differently.

Screenshot of the fix:
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-slowscript-fix.png

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
https://hg.mozilla.org/mobile-browser/rev/aeb08b2119c8

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED with the security label on build:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20090923
Fennec/1.0b4pre