1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[:Felipe Gomes (needinfo for replies!)]

Since the plan is to no longer use the notification bar for when there are click-to-play plugins, the plan instead is to change the plugin icon (in the URL bar) to red, on a soft transition after some X-seconds delay. So red will mean that plugins are inactive, and gray will mean that they are active.

[Tracking Requested - why for this release]:
UX change for Click-to-Play Flash

Comment 1

[Chris Peterson [:cpeterson]]

Jacqueline's current UX proposal is:

1. WHEN PLUGIN IS BLOCKED: Instead of showing the icon as red when the plugin is blocked, we instead will show a blue indicator dot on a light grey plugin icon that flashes a few times upon page load.

2. WHEN PLUGIN IS ACTIVATED: If the user enables the plugin, the plugin icon notification dot will disappear and the icon will turn a darker grey.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

An update here: this is taking longer than expected because PopupNotifications (which handles the URLbar icon) really isn't well suited for handling per-tab state (i.e., some tabs would have the active and some tabs would have the inactive icon).

There are three options that I see:
- Actually use two different notifications, one for sites with active plugins, one for sites with inactive plugins.  This seems easy but there are things hooked up to the id of this icon that would need to be changed to handle whichever is being displayed
- Have browser-plugins.js listen for TabSelect and update the class accordingly
- Make PopupNotification able to handle per-tab CSS class changes

All three would work, neither are really nice. I'm trying them to see what looks cleaner to be upliftable.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: flashbadge.mov

This video shows how it looks as implemented.

The only thing from the design that I couldn't implement was the "cut-out" from the blue badge into the plugin icon. Making that work correctly with the different themes, and lwthemes, was gonna be really hard, so I don't want to waste time on that.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: [mq]: settimeoutlogging

Review commit: https://reviewboard.mozilla.org/r/157482/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157482/

Comment 5

[:Felipe Gomes (needinfo for replies!)]

PopupNotifications.jsm wasn't really well prepared to handle different stylings of the icons per tab. Some other cases (like webrtc notifications) handle their icons entirely on their own, just letting PopupNotification show the doorhanger. But this would be a much bigger change, so the simple solution was to let each notification's options carry an option for an extra attribute to set.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8886700 [details]
[mq]: settimeoutlogging

https://reviewboard.mozilla.org/r/157482/#review162630

::: browser/base/content/browser-plugins.js:303
(Diff revision 1)
> +      badge.removeAttribute("animate");
> +    }, { once: true });

Please remove the {once: true} and check that the event.animationName matches what you expect and that event.target.classList.contains() what you expect, and then remove the event listener manually.

::: browser/themes/shared/notification-icons.inc.css:265
(Diff revision 1)
> +
> +#plugins-notification-icon[extraAttr="inactive"] > #plugin-icon-badge[animate] {
> +  animation: blinkBadge 1000ms ease 0s 5 alternate both;
> +}
> +
> +@keyframes blinkBadge {

nit, the "blink-badge" form seems more common in our tree (vs blinkBadge).

::: browser/themes/shared/notification-icons.svg:103
(Diff revision 1)
>    <use id="screen-indicator" href="#screen-icon"/>
>    <use id="screen-blocked" class="blocked" href="#screen-icon" />
>    <use id="update" href="#update-icon" />
>  
>    <path id="strikeout" d="m 2,28 2,2 26,-26 -2,-2 z"/>
> +  <circle id="bluebadge" cx="27" cy="5" r="5"/>

Can we just call this "badge"? I'd prefer not to hardcode the color in the name.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

Review commit: https://reviewboard.mozilla.org/r/157492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157492/

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8886700 [details]
[mq]: settimeoutlogging

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157482/diff/1-2/

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8886705 [details]
Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

This is r=jaws. Fixed the nits, but MozReview lost the right patch assignment because the previous push included an unrelated patch.

I ran all of the plugin UI tests locally and will request Autoland on this.

Comment 10

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8886705 [details]
Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

https://reviewboard.mozilla.org/r/157492/#review162636

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5c8920481ce
Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated. r=Felipe

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e5c8920481ce

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8886705 [details]
Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

https://reviewboard.mozilla.org/r/157492/#review162696

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8886705 [details]
Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

Approval Request Comment
[Feature/Bug causing the regression]: UX improvement for the release of Flash Click-To-Play in 55.
[User impact if declined]: Users will see less notice that there's a flash plugin to be activated
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: It would be good to, but I don't think we can wait for that before uplifting.. I'll organize QE ASAP and if there are any problems we could back out.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not much.. only contained to the styling of the plugin icon in the url bar.
[Why is the change risky/not risky?]: see above
[String changes made/needed]: none

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Hi Stefan, could you help us test this change?

With the latest Nightly, please do some exploratory work of websites with Flash to notice if the right icon in the URL bar is displayed. If Flash is active, the icon should be the one you're used to. If Flash is inactive, it should have a lighter gray color and a blue badge.

- Test this both in the default setting ("Ask to Activate") and the old ("Always Activate") Flash setting.

- Test that opening a site will correctly display it correctly active/inactive from the first time. This includes testing that a site that was marked as "Allow and remember" will correctly show the active icon from the beginning.

- Test that having tabs open with sites in all three cases (a site with no flash, a site with inactive flash, and a site with active flash), and switching these tabs will correctly update the icon (or hide it in the case of no flash).

- Test that allowing Flash on a site will correctly transition it to the active state.

- Test that when a site with inactive flash first loads, the badge will blink a couple of times. It will not blink when switching tabs.

- Test this with both websites that has visible Flash and hidden Flash..



Note: If you're uncertain if Flash is active on a website or not, you can click the icon and see what the doorhanger popup says..  "Would you like to allow"  or "Adobe Flash is enabled". (this patch didn't change anything related to the popup, so it can be treated as correct information for confirmation)

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 8886705 [details]
Bug 1379175 - Add a blue badge to the plugin icon in the URL bar when there are plugins to be activated.

plugin notification change for beta55, should be in 55.0b11

Comment 17

[Stefan [:StefanG_QA]]

(Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0) (20170718030207)

I've finished testing this feature on latest Nightly on Windows 10, OS X 10.12 and Ubuntu 17.04 in regular and private tabs. All tests has passed and no issues found. 

The blue badge correctly shows the transition to different state of flash for all tested pages. It blinks a couple of times when is shown for the first time on particular page and does not blink when switching tabs. If a page is set to "Allow and Remember" it shows the correct state when the page is loaded. 

I will test it on Beta 55.0b11 again once it is released.

Comment 18

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-beta/rev/af63b036dc7f