Closed
Bug 746110
Opened 12 years ago
Closed 12 years ago
Expose click-to-play plugins preference in the UI
Categories
(SeaMonkey :: Preferences, defect)
SeaMonkey
Preferences
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.13
People
(Reporter: iannbugzilla, Assigned: InvisibleSmiley)
References
Details
Attachments
(1 file, 1 obsolete file)
6.30 KB,
patch
|
neil
:
review+
iannbugzilla
:
ui-review+
|
Details | Diff | Splinter Review |
Bug 743312 implemented click-to-play plugins, we should probably expose the pref in SM's preferences UI.
Assignee | ||
Comment 1•12 years ago
|
||
Hmm, the Advanced/Scripts & Plugins pane doesn't have much space left, but I wonder whether we can make the "Enable Plugins for:" box half the size and thus gain some space for this new pref. Another option would be to put it under Appearance/Content (which is probably easier and not too wrong after all).
Whiteboard: [good first bug][mentor=InvisibleSmiley][lang=xul]
Comment 2•12 years ago
|
||
The "Allow scripts to" list is automatically resizeable and scrollable, so you can easily e.g. add a checkbox to the Enable Plugins groupbox.
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to comment #2) > The "Allow scripts to" list is automatically resizeable and scrollable, so you > can easily e.g. add a checkbox to the Enable Plugins groupbox. True, but it doesn't really fit /into/ that groupbox, does it? Read: "Enable plugins for:" ... "Require a click to activate plugins" - That's why I came up with the idea of putting a new groupbox to the right of the existing one. Otherwise, if we changed the groupbox label (caption), we'd probably have to change the labels of the two existing checkboxes. And then it would be /some much easier/ to just add a checkbox to the list on Appearance/Content. ;-)
Comment 4•12 years ago
|
||
I'm willing to work on this. Is this going in Appearance/Content or Advanced/Scripts & Plugins?
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Pranav Ravichandran [:pranavrc] from comment #4) > I'm willing to work on this. Great! :-) Just assign the bug to yourself (take). If you have any further questions (e.g. how to provide a patch or work with Bugzilla), feel free to contact me by mail or just add a comment here. > Is this going in Appearance/Content or > Advanced/Scripts & Plugins? Right now, both appear as valid options to me (see the discussion above). Ultimately, this is between the patch author (i.e. you) and the reviewer. Pick one (e.g. Neil from comment 2, or IanN) and either ask them on IRC (irc.mozilla.org, #seamonkey) upfront or implement it one way and see what the reviewer replies after you uploaded your patch and requested review here on this bug.
Updated•12 years ago
|
Assignee: nobody → prp.1111
Comment 6•12 years ago
|
||
(In reply to Jens Hatlak from comment #3) > (In reply to comment #2) > > The "Allow scripts to" list is automatically resizeable and scrollable, so you > > can easily e.g. add a checkbox to the Enable Plugins groupbox. > True, but it doesn't really fit /into/ that groupbox, does it? Ideally I'd want the checkbox to be disabled if the "Enable Plugins" checkbox is disabled, which is easier to do on the Scripts pane. Also, we could label it "Automatically activate plugins" (i.e. inverting the preference) which would make it fit more neatly into the groupbox.
Comment 7•12 years ago
|
||
Thinking about it I think maybe the word "always" wouldn't go amiss e.g. "Always activate all plugins". This is because sometimes plugins get activated even though click-to-play is on.
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•12 years ago
|
||
Pranav, what's the status?
Assignee | ||
Comment 9•12 years ago
|
||
No reaction to comment 8, taking over. I took a slightly different approach, looking at it from the "page requires plugins" POV, thus grouping the new preference with the existing one for notification bars. Also I find this pref is pretty much independent from the general plugins enabling pref since it mirrors a user preference, just like the one for notification bars. Hence no JS disabling code needed. The notification bars pref label is now a bit longer and wraps, but I feel that's OK. The only thing that worries me a bit is that the "Allow scripts to:" list (which now shows a slider by default) cannot be scrolled down completely, at least for me on Win7. :-/ While I was it I also removed the colon from the "Enable Plugins for" caption - we're not doing that anywhere else AFAICS (esp. on the same pane, "Enable JavaScript for").
Assignee: prp.1111 → jh
Attachment #633921 -
Flags: review?(neil)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [good first bug][mentor=InvisibleSmiley][lang=xul]
Comment 10•12 years ago
|
||
Comment on attachment 633921 [details] [diff] [review] patch The "Allow scripts to:" listbox displayed fine for me on Windows 8 Preview, even when I resized the preferences window. >-<!ENTITY enablePlugin.label "Enable Plugins for:"> >+<!ENTITY enablePlugins.label "Enable Plugins for"> Nit: too much space >+<!ENTITY displayNotification2.label "Display a notification bar at the top of the content area if additional plugins are required"> Maybe this should say "Warn me if additional plugins need to be installed"? (It might also help you give the entity a nicer name.)
Attachment #633921 -
Flags: review?(neil) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Sorry, forgot the Help changes. Thankfully accepted your suggestion - the scroll bars are gone! :-)
Attachment #633921 -
Attachment is obsolete: true
Attachment #634079 -
Flags: review?(neil)
Comment 12•12 years ago
|
||
Comment on attachment 634079 [details] [diff] [review] patch incl. Help [Checkin: Comment 17] > <checkbox id="displayPluginsNotification" >- label="&displayNotification.label;" >- accesskey="&displayNotification.accesskey;" >+ label="&warnPluginsRequired.label;" >+ accesskey="&warnPluginsRequired.accesskey;" Nit: change the id of the checkbox to "warnPluginsRequired" to match? > <li><strong>Enable Plugins for</strong>: Use these checkboxes to control how > plugins are used: Interestingly the help does include a colon (but outside of the bold) ;-) >+ You can activate individual plugin instances with a left mouse click, Might be worth adding the word visible somewhere? >+ or activate all plugins on the page using the doorhanger (or >+ notification bar). The latter also gives you the option to do the Not sure that notification bar needs ()s? >+ activtaion either temporarily or remember your choice for the current >+ website. Remembered choices can be edited using the Data Manager >+ (Permissions tab).</li> Aside from the misspelling of activation, this doesn't quite sound right to me, although the best I can do is a rather overlong sentence. IanN? You can activate individual visible plugin instances with a left mouse click or use the notification to activate all the plugins on the page, optionally remembering your choice for the current website.
Attachment #634079 -
Flags: ui-review?(iann_bugzilla)
Attachment #634079 -
Flags: review?(neil)
Attachment #634079 -
Flags: review+
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #12) > >+ or activate all plugins on the page using the doorhanger (or > >+ notification bar). The latter also gives you the option to do the > Not sure that notification bar needs ()s? Without the parentheses, "The latter" does not refer to what I intended - both provide the option, not just the notification bar.
Reporter | ||
Comment 14•12 years ago
|
||
How about: You can activate individual visible plugin instances with a left mouse click or use the notification to activate all the plugins on the page. From the notification you have the option to either temporarily activate the plugins or remember the choice for the current website. Remembered choices can be edited using the Data Manager (Permissions tab).
Assignee | ||
Comment 15•12 years ago
|
||
Ian, I'm mostly fine with your suggestion, but it occurred to me that just saying "notification" is a bit too vague in case of the doorhanger, because it doesn't appear by itself (as opposed to e.g. the doorhangers for add-on downloads). I've missed the small plugin icon in the location bar myself more than once. The best I could come up with goes something like this: "... Otherwise, if the page requires plugins, &brandShortName; will show a notification (plugin icon in the location bar or notification bar), plus a placeholder for every plugin instance found on the page (unless the website is whitelisted, see below)." What do the two of you think?
Attachment #634079 -
Flags: ui-review?(iann_bugzilla) → ui-review+
Reporter | ||
Comment 16•12 years ago
|
||
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #15) > Ian, I'm mostly fine with your suggestion, but it occurred to me that just > saying "notification" is a bit too vague in case of the doorhanger, because > it doesn't appear by itself (as opposed to e.g. the doorhangers for add-on > downloads). I've missed the small plugin icon in the location bar myself > more than once. > > The best I could come up with goes something like this: > > "... Otherwise, if the page requires plugins, &brandShortName; will show a > notification (plugin icon in the location bar or notification bar), plus a > placeholder for every plugin instance found on the page (unless the website > is whitelisted, see below)." > > What do the two of you think? Seems good to me.
Assignee | ||
Comment 17•12 years ago
|
||
Comment on attachment 634079 [details] [diff] [review] patch incl. Help [Checkin: Comment 17] http://hg.mozilla.org/comm-central/rev/f9abd1dbd599 (with review comments addressed)
Attachment #634079 -
Attachment description: patch incl. Help → patch incl. Help [Checkin: Comment 17]
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
You need to log in
before you can comment on or make changes to this bug.
Description
•