Closed
Bug 896418
Opened 11 years ago
Closed 11 years ago
Improve naming for CtP popupNotification centerActions
Categories
(Core Graveyard :: Plug-ins, enhancement, P3)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla30
People
(Reporter: gfritzsche, Assigned: gfritzsche)
References
Details
Attachments
(1 file)
16.72 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
The centerActions naming for the new CtP doorhanger code could probably be named more clearly. This affects:
* gPluginHandler._makeCenterActions()
* ctpNotification.options.centerActions
The centerActions are used by browser-plugins.js to set the click-to-play plugin data on the notification from which the code in urlbarBindings.xml builds it state.
Assignee | ||
Comment 1•11 years ago
|
||
Given the usage, how about pluginActions or pluginData instead?
Comment 2•11 years ago
|
||
This was mainly historical from the old doorhanger. `pluginData` is good, but please let's delay this until bug 888510 lands.
Comment 3•11 years ago
|
||
There are also various mentions of a mysterious "center item"... http://mxr.mozilla.org/mozilla-central/search?string=center-item
Comment 4•11 years ago
|
||
I'm not sure what's so mysterious about the UI elements, which are in fact the center items of the multi-plugin dialog.
Comment 5•11 years ago
|
||
Whether they're visually centered or not seems pretty much irrelevant for the code using that name. A name describing the purpose of these items would be more useful.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → georg.fritzsche
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #8377110 -
Flags: review?(jaws)
Comment 7•11 years ago
|
||
Comment on attachment 8377110 [details] [diff] [review]
Rename centerActions to pluginData
Review of attachment 8377110 [details] [diff] [review]:
-----------------------------------------------------------------
I double-checked and couldn't find any other references to centerActions.
::: browser/base/content/browser-plugins.js
@@ +888,5 @@
> url = Services.urlFormatter.formatURLPref("app.support.baseURL") + "clicktoplay";
> }
> pluginInfo.detailsLink = url;
>
> + pluginData.set(pluginInfo.permissionString, pluginInfo);
Can you fix this trailing whitespace while you're in here?
Attachment #8377110 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) [PTO 2/20-2/23] from comment #7)
> Can you fix this trailing whitespace while you're in here?
Oh, sure, i missed that.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb77326af133
Comment 9•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•