Closed
Bug 756025
Opened 12 years ago
Closed 12 years ago
Port |Bug 742753 - Click to Play should permit each element|
Categories
(SeaMonkey :: UI Design, defect)
SeaMonkey
UI Design
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.12
People
(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)
Details
Attachments
(1 file, 1 obsolete file)
2.39 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
Clicking a single plugin area in CTP mode should only enable that plugin instance and not all the others on the current page (activating all should still be possible using the doorhanger or notification bar).
Assignee | ||
Comment 1•12 years ago
|
||
Test case: attachment 612565 [details]
Attachment #624672 -
Flags: review?(neil)
Comment 2•12 years ago
|
||
Comment on attachment 624672 [details] [diff] [review]
patch
>+ var haveUnplayedPlugins = this.contentWindowUtils.plugins.some(function(plugin) {
>+ let objLoadingContent = plugin.QueryInterface(Components.interfaces.nsIObjectLoadingContent);
Nit: let at top-level function scope ;-) [but see below]
>+ return (plugin != aPlugin && !objLoadingContent.activated);
Nit: unnecessary ()s. [Also, confusing variable names or what?]
>+ var notification = PopupNotifications.getNotification("click-to-play-plugins",
>+ this.activeBrowser);
>+ if (notification && !haveUnplayedPlugins) {
>+ this.clickToPlayPromptShown = false;
>+ notification.remove();
>+ }
You need to call removePluginClickToPlayPrompt instead of manually removing the prompt, so it will work for both notificationboxes and doorhangers. (Of course you still need to check for haveUnplayedPlugins first.)
I wonder whether we should set the prompt shown variable to false.
Attachment #624672 -
Flags: review?(neil) → review-
Assignee | ||
Comment 3•12 years ago
|
||
> I wonder whether we should set the prompt shown variable to false.
I'm not sure what you meant - moving that to the remove method? For now I interpreted it as "maybe we don't want to set it to false in this case at all", reflected in the updated patch.
Attachment #624672 -
Attachment is obsolete: true
Attachment #625049 -
Flags: review?(neil)
Comment 4•12 years ago
|
||
(In reply to Jens Hatlak from comment #3)
> > I wonder whether we should set the prompt shown variable to false.
> I'm not sure what you meant - moving that to the remove method?
Oversight on my part. Put it back wherever you like.
Comment 5•12 years ago
|
||
Comment on attachment 625049 [details] [diff] [review]
patch [Checkin: Comment 6]
[There seems to be a core bug here - I see entries for plugins that won't start (e.g. object/embed fallbacks generate two entries) but I eventually found a page that just had one embed and clicking it did remove the doorhanger.]
Attachment #625049 -
Flags: review?(neil) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 625049 [details] [diff] [review]
patch [Checkin: Comment 6]
http://hg.mozilla.org/comm-central/rev/f5b07be535e1
(with re-addition of this.clickToPlayPromptShown = false;)
Attachment #625049 -
Attachment description: patch → patch [Checkin: Comment 6]
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #5)
> [There seems to be a core bug here - I see entries for plugins that won't
> start (e.g. object/embed fallbacks generate two entries) but I eventually
> found a page that just had one embed and clicking it did remove the
> doorhanger.]
I also found it strange that clicking all three plugin areas from the test case from comment 1 (which is actually from the base bug) did not remove the doorhanger, but at least it's consistent with FF. ;-) I will closely monitor all CTP-related bug fixes and see what happens there.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12
You need to log in
before you can comment on or make changes to this bug.
Description
•