Closed Bug 896418 Opened 6 years ago Closed 6 years ago

Improve naming for CtP popupNotification centerActions

Categories

(Core :: Plug-ins, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: gfritzsche, Assigned: gfritzsche)

References

Details

Attachments

(1 file)

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.
Given the usage, how about pluginActions or pluginData instead?
This was mainly historical from the old doorhanger. `pluginData` is good, but please let's delay this until bug 888510 lands.
There are also various mentions of a mysterious "center item"... http://mxr.mozilla.org/mozilla-central/search?string=center-item
I'm not sure what's so mysterious about the UI elements, which are in fact the center items of the multi-plugin dialog.
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.
Depends on: 888510
Assignee: nobody → georg.fritzsche
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+
(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
https://hg.mozilla.org/mozilla-central/rev/bb77326af133
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.