The default bug view has changed. See this FAQ.

Add addon-options-displayed/addon-options-hidden as constants on AddonManager

RESOLVED FIXED in mozilla18

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: Julian)

Tracking

({dev-doc-needed})

Trunk
mozilla18
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js])

Attachments

(2 attachments, 4 obsolete attachments)

The addon-options-displayed and addon-options-hidden notifications are used for inline settings in the UI. They're starting to be used by a couple of providers to add provider-specific UI (bug 335781, bug 619652). At the moment, they're just magic hard-coded values - since they're being re-used more (and also used in addons), they should become re-usable constants.

They should be named OPTIONS_NOTIFICATION_DISPLAYED and OPTIONS_NOTIFICATION_HIDDEN, and be added after the OPTIONS_TYPE_* constants on the AddonManager object in AddonManager.jsm:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm


List of existing uses that should be updated to use the new constants:
https://mxr.mozilla.org/mozilla-central/search?string=addon-options-&regexp=1&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Blocks: 619652, 335781
Assignee: nobody → jmuenster
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 664086 [details] [diff] [review]
Partial Patch File Toolkit
(Assignee)

Comment 2

5 years ago
Created attachment 664087 [details] [diff] [review]
Partial Patch File Mobile
Comment on attachment 664086 [details] [diff] [review]
Partial Patch File Toolkit

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

I'll be extra pedantic, since you asked :)

When attaching a patch, you can (and should) request review from someone, so it doesn't get lost. In the attachment upload page, there are a series of flags. Set the review flag to "?", and enter the email of whoever you think should review the patch (can start entering a name, and it'll try to autocomplete). https://wiki.mozilla.org/Modules has a list of reviewers, or just look at whoever review code changes there recently (for the Add-ons Manager, its safe to assume its me).

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +2200,5 @@
>    // Options will be displayed in a new tab, if possible
>    OPTIONS_TYPE_TAB: 3,
>  
> +  // Constants for displayed or hidden options notifications
> +  // Options notification will be displayed 

Careful about accidentally inserting trailing whitespace.

::: toolkit/mozapps/extensions/content/extensions.js
@@ +2878,5 @@
>            gDetailView.node.addEventListener("ViewChanged", function viewChangedEventListener() {
>              gDetailView.node.removeEventListener("ViewChanged", viewChangedEventListener, false);
>              if (firstSetting)
>                firstSetting.clientTop;
> +            Services.obs.notifyObservers(document, AddonManager.OPTIONS_NOTIFICATION_DISPLAYED, gDetailView._addon.id);

Should try to wrap lines to 80 characters, and be sure to indent using spaces not tabs (see the other calls to addObserver in this patch).
See https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style
Attachment #664086 - Flags: review-
Comment on attachment 664087 [details] [diff] [review]
Partial Patch File Mobile

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

Should fix the long lines here too.
Attachment #664087 - Flags: review-
(Assignee)

Comment 5

5 years ago
Created attachment 665371 [details] [diff] [review]
Partial Patch File Mobile
Attachment #664087 - Attachment is obsolete: true
Attachment #665371 - Flags: review?(bmcbride)
(Assignee)

Comment 6

5 years ago
Created attachment 665374 [details] [diff] [review]
Partial Patch File Toolkit
Attachment #664086 - Attachment is obsolete: true
Attachment #665374 - Flags: review?(bmcbride)
(Assignee)

Comment 7

5 years ago
Created attachment 665447 [details] [diff] [review]
Partial Patch File Mobile
Attachment #665371 - Attachment is obsolete: true
Attachment #665371 - Flags: review?(bmcbride)
Attachment #665447 - Flags: review?(bmcbride)
(Assignee)

Comment 8

5 years ago
Created attachment 665448 [details] [diff] [review]
Partial Patch File Toolkit
Attachment #665374 - Attachment is obsolete: true
Attachment #665374 - Flags: review?(bmcbride)
Attachment #665448 - Flags: review?(bmcbride)
Attachment #665447 - Flags: review?(bmcbride) → review+
Attachment #665448 - Flags: review?(bmcbride) → review+
Landed on the fx-team branch, which will get merged into mozilla-central within a day or so:
https://hg.mozilla.org/integration/fx-team/rev/cd72779323e1
Keywords: dev-doc-needed
Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js] → [good first bug][mentor=bmcbride@mozilla.com][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cd72779323e1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js][fixed-in-fx-team] → [good first bug][mentor=bmcbride@mozilla.com][lang=js]
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.