Closed Bug 756025 Opened 10 years ago Closed 10 years ago

Port |Bug 742753 - Click to Play should permit each element|

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.12

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Details

Attachments

(1 file, 1 obsolete file)

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).
Attached patch patch (obsolete) — Splinter Review
Test case: attachment 612565 [details]
Attachment #624672 - Flags: review?(neil)
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-
> 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)
(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 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+
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]
(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: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12
You need to log in before you can comment on or make changes to this bug.