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

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

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).

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Test case: attachment 612565 [details]

Comment 2

[[e-mail]]

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.

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch [Checkin: Comment 6]

> 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.

Comment 4

[[e-mail]]

(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

[[e-mail]]

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.]

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

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;)

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] 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.