The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in seamonkey2.12

Status

SeaMonkey
UI Design
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.12

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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

5 years ago
Created attachment 624672 [details] [diff] [review]
patch

Test case: attachment 612565 [details]
Attachment #624672 - Flags: review?(neil)

Comment 2

5 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

5 years ago
Created attachment 625049 [details] [diff] [review]
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.
Attachment #624672 - Attachment is obsolete: true
Attachment #625049 - Flags: review?(neil)

Comment 4

5 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

5 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

5 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

5 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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12
You need to log in before you can comment on or make changes to this bug.