De-uglify references to prefpane variables

RESOLVED FIXED in seamonkey2.0a1


10 years ago
9 years ago


(Reporter: Robert Kaiser, Assigned:



Firefox Tracking Flags

(Not tracked)



(1 attachment)



10 years ago
Currently, we need to reference prefpane variables with document.getElementById('foo_pane').* when they come from XBL, which includes the helper app listbox stuff and even the menuitems added to it via JS, but also onsyncfrompreference attributes that actually get called through the toolkit preference XBL. We should look into "de-uglification" of that, as Neil named it in review comments to bug 417590.

Comment 1

10 years ago
Created attachment 308332 [details] [diff] [review]
Fix pref-applications

For KaiRo, just looking to double-check I didn't break anything ;-)

* Removed the XUL event listeners and added them in JS instead [optional]
* Moved code to handleEvent to cope with the extra events and targets
* Swapped from alwaysAsk and action attributes to value attributes
* Swapped from var to let, because we shouldn't have used the vars again anyway
* Added two new action types for chooseApp and manageApp
* Simplified the code to set the initial menuitem using its value
* Simplified the code to store the alwaysAsk setting
* Fired events from XBL to avoid calling global methods
Attachment #308332 - Flags: superreview?(jag)
Attachment #308332 - Flags: review?(kairo)

Comment 2

10 years ago
Comment on attachment 308332 [details] [diff] [review]
Fix pref-applications

>Index: suite/common/pref/pref-applications.js

> // But since nsIHandlerInfo doesn't support plugins, there's no value
> // identifying the "use plugin" action, so we use this constant instead.
> const kActionUsePlugin = 5;
>+const kActionChooseApp = 6;
>+const kActionManageApp = 7;

Could we maybe change these to e.g. 1001, 1002, and 1003 so there's less of a chance of a collision? Or maybe 0x101, 0x102, 0x103, if you're into that sort of thing.

>@@ -1462,26 +1502,13 @@
>     // the item identified by the preferred action (when the preferred action
>     // is to use a helper app, we have to pick the specific helper app item).
>     if (handlerInfo.alwaysAskBeforeHandling)
>+      menu.value = nsIHandlerInfo.alwaysAsk;
>+    else if (handlerInfo.preferredAction == nsIHandlerInfo.userHelperApp &&
>+             preferredApp)

Typo: use, not user, in useHelperApp.

sr=jag with these addressed
Attachment #308332 - Flags: superreview?(jag) → superreview+

Comment 3

10 years ago
Comment on attachment 308332 [details] [diff] [review]
Fix pref-applications

Seems to be working fine here, from what I see.
Attachment #308332 - Flags: review?(kairo) → review+

Comment 4

10 years ago
Fix checked in.


10 years ago
Last Resolved: 10 years ago
Resolution: --- → FIXED


9 years ago
Assignee: nobody → neil
Target Milestone: --- → seamonkey2.0a1
You need to log in before you can comment on or make changes to this bug.