Closed Bug 1043372 Opened 10 years ago Closed 10 years ago

Figure out keyboard-accessible alternative for the webrtc global indicator

Categories

(Firefox :: Keyboard Navigation, defect)

defect
Not set
normal
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.3

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Keywords: access, Whiteboard: [bugday-20140820])

Attachments

(1 file, 4 obsolete files)

The current global indicator can't be focused with the keyboard. This means that a keyboard user can only manually search for the sharing tab and use tab navigation to activate the doorhanger to stop sharing.

We should figure out something better to do, like:

- use registerhotkey ( http://msdn.microsoft.com/en-ca/library/windows/desktop/ms646309%28v=vs.85%29.aspx ) on Windows to provide a hotkey to access the indicator, and then rely on simple tab navigation inside the hotkey.

Downside: how do you expose the hotkey? Can we do equivalent things on other platforms? How do we pick a non-conflicting hotkey?

- add a menu somewhere (under "View"? "Tools" ? "Edit"?) which lists the sharing tabs so the user can quickly find them.
Downside: relies on the old-style menu

- add a visual indicator to make it easier to spot tabs that are sharing content 

Downside: needs more visual assets, tabs are already small and so the indicators might crowd out text, and would still need a lot of scrolling manually to find the offending tabs if you have many tabs open.

Gavin, can we get this on the roadmap so we fix this for 34 even if it misses the 33 boat due to l10n? :-)

Philipp, can you think of another option that I've missed and/or indicate what solution you'd prefer from a UX perspective?
Flags: needinfo?(philipp)
Flags: needinfo?(gavin.sharp)
Blocks: 1042163, 1040061
Whiteboard: [sceensharing-uplift]
Adding a visual indicator is something we should do regardless of the keyboard discussion.

Utilizing the classic menu also seems appropriate, since I would assume that lots of keyboard-only users still rely on it. We could have a section under the »Window« menu that is disabled/hidden by default and lists sharing tabs when they are present.
Flags: needinfo?(philipp)
Adding a tab indicator is a roundabout way of fixing the keyboard accessibility feature. That should probably be tracked in a separate bug - Philipp, do you want to file that?

For this bug, adding a keyboard-only menu to the classic menu seems reasonable. I've added it to the backlog for 34.2.
Flags: needinfo?(gavin.sharp)
Flags: firefox-backlog+
(In reply to Philipp Sackl [:phlsa] from comment #1)
> Adding a visual indicator is something we should do regardless of the
> keyboard discussion.
> 
> Utilizing the classic menu also seems appropriate, since I would assume that
> lots of keyboard-only users still rely on it. We could have a section under
> the »Window« menu that is disabled/hidden by default and lists sharing tabs
> when they are present.

This sounds fair enough to me, but the Window menu is OS X only. What menu could/should we use on Linux/Windows - and if we use something else, should we put it in the same place on OS X for consistency? (I guess we could put it under the "View" or "Tools" menu - or we could dynamically create a separate menu...)
(In reply to :Gijs Kruitbosch (Gone July 26 - August 3) from comment #3)
> (In reply to Philipp Sackl [:phlsa] from comment #1)
> > Adding a visual indicator is something we should do regardless of the
> > keyboard discussion.
> > 
> > Utilizing the classic menu also seems appropriate, since I would assume that
> > lots of keyboard-only users still rely on it. We could have a section under
> > the »Window« menu that is disabled/hidden by default and lists sharing tabs
> > when they are present.
> 
> This sounds fair enough to me, but the Window menu is OS X only. What menu
> could/should we use on Linux/Windows - and if we use something else, should
> we put it in the same place on OS X for consistency? (I guess we could put
> it under the "View" or "Tools" menu - or we could dynamically create a
> separate menu...)
Flags: needinfo?(philipp)
(In reply to :Gijs Kruitbosch (Bugmail catchup, needinfo if urgent) from comment #4)
> (In reply to :Gijs Kruitbosch (Gone July 26 - August 3) from comment #3)
> > (In reply to Philipp Sackl [:phlsa] from comment #1)
> > > Adding a visual indicator is something we should do regardless of the
> > > keyboard discussion.
> > > 
> > > Utilizing the classic menu also seems appropriate, since I would assume that
> > > lots of keyboard-only users still rely on it. We could have a section under
> > > the »Window« menu that is disabled/hidden by default and lists sharing tabs
> > > when they are present.
> > 
> > This sounds fair enough to me, but the Window menu is OS X only. What menu
> > could/should we use on Linux/Windows - and if we use something else, should
> > we put it in the same place on OS X for consistency? (I guess we could put
> > it under the "View" or "Tools" menu - or we could dynamically create a
> > separate menu...)

Oh, I didn't realize that it was OS X only. Thanks for catching that!
Creating a new menu dynamically sounds like the best option then because it doesn't really fit well anywhere else. This also ensures that it has a better visibility when it is important.
Let's put it between Tools and Help.

I would still use the Window menu on OS X though since
- it fits there naturally
- the menu bar is always visible on OS X, so adding a new item increases the clutter quite a bit
Flags: needinfo?(philipp)
Points: --- → 5
QA Whiteboard: [qa+]
Florian, does the whiteboard tag mean what I think it means? Because if we're using menus we'll need a string for the (sub)menu names and so on, and that will involve l10n, and so no possibility of uplifting for 33... if that's an issue, who do we discuss with, and could we / should we do this in a way that doesn't involve strings?


(In reply to Philipp Sackl [:phlsa] from comment #5)
> Oh, I didn't realize that it was OS X only. Thanks for catching that!
> Creating a new menu dynamically sounds like the best option then because it
> doesn't really fit well anywhere else. This also ensures that it has a
> better visibility when it is important.
> Let's put it between Tools and Help.
> 
> I would still use the Window menu on OS X though since
> - it fits there naturally
> - the menu bar is always visible on OS X, so adding a new item increases the
> clutter quite a bit

OK, so if this is what we're doing, I'm assuming this is the implementation bug, or should this still be a breakdown or something else? I don't know how/why this was put at 5 points otherwise without changing it to a breakdown, and keeping [qa+]

With that in mind, here are some questions before this can really be implemented:
1) see earlier paragraph
2) if we're sticking with names, what names for the submenu (for the Window menu) or toplevel menu (for Windows/Linux), and how do we indicate what each tab is sharing? Creating a double-layered sub-submenu (ie "Sharing > Audio > Page 1, Page 2; Sharing > Window > Page 3, Page 4") seems clumsy.. we could perhaps use "headers" by using disabled menu items and separate sections that way? Or just no labels and show the whole list and let the user figure out what is shared where?
I think I prefer the disabled-menus-as-section-headers option, but I'm not the UX person here --> ni for phlsa.
Assignee: nobody → gijskruitbosch+bugs
Iteration: --- → 34.2
Flags: needinfo?(philipp)
Flags: needinfo?(florian)
(In reply to :Gijs Kruitbosch (Bugmail catchup, needinfo if urgent) from comment #6)
> Florian, does the whiteboard tag mean what I think it means?

I think it was added semi-automatically at the same time as it was added on all dependencies of bug 1040061. I don't think uplifting this to 33 is required. We really want to have something accessible for 34 though :-).
Flags: needinfo?(florian)
Whiteboard: [sceensharing-uplift]
Status: NEW → ASSIGNED
QA Contact: drno
(In reply to :Gijs Kruitbosch from comment #6)
> OK, so if this is what we're doing, I'm assuming this is the implementation
> bug, or should this still be a breakdown or something else? I don't know
> how/why this was put at 5 points otherwise without changing it to a
> breakdown, and keeping [qa+]

I think this is an implementation bug. If you think this needs a breakdown we can morph it. It might make sense to break this into separate OSX and Win/Linux pieces.

> With that in mind, here are some questions before this can really be
> implemented:
> 1) see earlier paragraph

Not sure which paragraph you're referring to.

> 2) if we're sticking with names, what names for the submenu (for the Window
> menu) or toplevel menu (for Windows/Linux), and how do we indicate what each
> tab is sharing? Creating a double-layered sub-submenu (ie "Sharing > Audio >
> Page 1, Page 2; Sharing > Window > Page 3, Page 4") seems clumsy.. we could
> perhaps use "headers" by using disabled menu items and separate sections
> that way? Or just no labels and show the whole list and let the user figure
> out what is shared where?
> I think I prefer the disabled-menus-as-section-headers option, but I'm not
> the UX person here --> ni for phlsa.

I certainly defer to Philipp on this, but how about just a list of tab titles with the shared media appended? Something like:

Shared:
  "NYTimes.com Interactive Video: The Search for Yeti" is sharing video and audio
  "Skype.com - Phone Call" is sharing audio

I.e. a "%S is sharing %S" structure (I imagine we already have similar strings elsewhere?).
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #8)
> (In reply to :Gijs Kruitbosch from comment #6)
> > OK, so if this is what we're doing, I'm assuming this is the implementation
> > bug, or should this still be a breakdown or something else? I don't know
> > how/why this was put at 5 points otherwise without changing it to a
> > breakdown, and keeping [qa+]
> 
> I think this is an implementation bug. If you think this needs a breakdown
> we can morph it. It might make sense to break this into separate OSX and
> Win/Linux pieces.
> 
> > With that in mind, here are some questions before this can really be
> > implemented:
> > 1) see earlier paragraph
> 
> Not sure which paragraph you're referring to.

The paragraph in the same comment about whether we should use strings or use icons or something else that would be less string-intensive and potentially upliftable.


> > 2) if we're sticking with names, what names for the submenu (for the Window
> > menu) or toplevel menu (for Windows/Linux), and how do we indicate what each
> > tab is sharing? Creating a double-layered sub-submenu (ie "Sharing > Audio >
> > Page 1, Page 2; Sharing > Window > Page 3, Page 4") seems clumsy.. we could
> > perhaps use "headers" by using disabled menu items and separate sections
> > that way? Or just no labels and show the whole list and let the user figure
> > out what is shared where?
> > I think I prefer the disabled-menus-as-section-headers option, but I'm not
> > the UX person here --> ni for phlsa.
> 
> I certainly defer to Philipp on this, but how about just a list of tab
> titles with the shared media appended? Something like:
> 
> Shared:
>   "NYTimes.com Interactive Video: The Search for Yeti" is sharing video and
> audio
>   "Skype.com - Phone Call" is sharing audio
> 
> I.e. a "%S is sharing %S" structure (I imagine we already have similar
> strings elsewhere?).

I don't know if we do, but reusing them wouldn't be an option, I guess... 

The other thing is that menus don't really work well for really long text like page titles - the bookmarks and history menu certainly both crop at a certain length. We'd want to do a similar "middle crop" - but only on the page title, because we'd want to make sure that the "is sharing video and audio" bit isn't cropped (and we can't just left/right crop because RTL, and trying to figure out the direction of text for each of the page titles isn't really a rabbit hole I think we should go down here). This is tricky because I don't know how different OSes crop the text (particularly OS X as compared to the others, as it has native menus), and how font sizes will come in there if the user customizes them. Maybe I'm seeing "bears on the road" (Dutch figure of speech, I'm afraid) too early, but I'd ideally want to avoid having a composed string inside the menu...
(In reply to :Gijs Kruitbosch from comment #9)
> (In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #8)
> > > 2) if we're sticking with names, what names for the submenu (for the Window
> > > menu) or toplevel menu (for Windows/Linux), and how do we indicate what each
> > > tab is sharing? Creating a double-layered sub-submenu (ie "Sharing > Audio >
> > > Page 1, Page 2; Sharing > Window > Page 3, Page 4") seems clumsy.. we could
> > > perhaps use "headers" by using disabled menu items and separate sections
> > > that way? Or just no labels and show the whole list and let the user figure
> > > out what is shared where?
> > > I think I prefer the disabled-menus-as-section-headers option, but I'm not
> > > the UX person here --> ni for phlsa.
> > 
> > I certainly defer to Philipp on this, but how about just a list of tab
> > titles with the shared media appended? Something like:
> > 
> > Shared:
> >   "NYTimes.com Interactive Video: The Search for Yeti" is sharing video and
> > audio
> >   "Skype.com - Phone Call" is sharing audio
> > 
> > I.e. a "%S is sharing %S" structure (I imagine we already have similar
> > strings elsewhere?).
> 
> I don't know if we do, but reusing them wouldn't be an option, I guess... 
> 
> The other thing is that menus don't really work well for really long text
> like page titles - the bookmarks and history menu certainly both crop at a
> certain length. We'd want to do a similar "middle crop" - but only on the
> page title, because we'd want to make sure that the "is sharing video and
> audio" bit isn't cropped (and we can't just left/right crop because RTL, and
> trying to figure out the direction of text for each of the page titles isn't
> really a rabbit hole I think we should go down here). This is tricky because
> I don't know how different OSes crop the text (particularly OS X as compared
> to the others, as it has native menus), and how font sizes will come in
> there if the user customizes them. Maybe I'm seeing "bears on the road"
> (Dutch figure of speech, I'm afraid) too early, but I'd ideally want to
> avoid having a composed string inside the menu...

The idea of using one simple list is great!
How about just using the domain to indicate the origin?

Menu title: »Tabs sharing devices«
Items: »webex.com (camera, microphone and screen)«

That could lead to the hypothetical situation of two items with the same text but that is so much of an edge case that I think it would be OK to go with it.
Flags: needinfo?(philipp)
Here's a WIP... this seems to work fine on OS X; going to test Windows next.
Had to tweak the accesskey on Windows, otherwise this seems to work right out of the box on Windows, too. I've removed the explicit OSX-specific accesskey because I expect that it'll confuse l10n tools and isn't really necessary anyway (as there's no visual indication on OSX, and it'll just use the first letter, which was what the access key was anyway (because it was in the Window menu - can't be the same on Windows/Linux because the Tools menu already has the 'T' accesskey; can't use 's' because it's in use by History...)
Attachment #8470014 - Flags: review?(jaws)
Attachment #8469995 - Attachment is obsolete: true
Attachment #8470014 - Flags: ui-review?(philipp)
remote:   https://tbpl.mozilla.org/?tree=Try&rev=61fc5b038557
remote: Alternatively, view them on Treeherder (experimental):
remote:   https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=61fc5b038557
Comment on attachment 8470014 [details] [diff] [review]
sharing indicator in main menu for keyboard accessibility,

Looks good on OS X!
Attachment #8470014 - Flags: ui-review?(philipp) → ui-review+
(In reply to :Gijs Kruitbosch from comment #13)
> remote:   https://tbpl.mozilla.org/?tree=Try&rev=61fc5b038557
> remote: Alternatively, view them on Treeherder (experimental):
> remote:  
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=61fc5b038557

Fully green try... been a while since I've seen one of those!
Comment on attachment 8470014 [details] [diff] [review]
sharing indicator in main menu for keyboard accessibility,

Review of attachment 8470014 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/browser.properties
@@ +536,5 @@
> +getUserMedia.sharingMenu.label = Tabs sharing devices
> +getUserMedia.sharingMenu.accesskey = d
> +getUserMedia.sharingMenuCamera = %S (camera)
> +getUserMedia.sharingMenuMicrophone = %S (microphone)
> +getUserMedia.sharingMenuScreen = %S (screen)

There should be localization notes above each of these describing what the %S will be replaced with.

::: browser/modules/webrtcUI.jsm
@@ +581,5 @@
> +      tabIdentifier = streamInfo.browser.currentURI.host;
> +    } catch (ex) {};
> +    if (!tabIdentifier) {
> +      // This is unfortunate, but we should display *something*...
> +      tabIdentifier = streamInfo.browser.currentURI.scheme + ":...";

So this could potentially show "https:..." ? I don't see that as being useful. We should probably have some defined fallback text that says "(host unknown, proceed with caution)"
Attachment #8470014 - Flags: review?(jaws) → review+
Comment on attachment 8470014 [details] [diff] [review]
sharing indicator in main menu for keyboard accessibility,

Review of attachment 8470014 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/webrtcUI.jsm
@@ +577,5 @@
> +
> +    let tabIdentifier;
> +    try {
> +      // This can throw for some types of URLs.
> +      tabIdentifier = streamInfo.browser.currentURI.host;

This will show the hostname of the tab's top level document rather than the host actually using the device (which could be a different one if there are frames involved).

We used to have this behavior, until it was changed in a security bug (bug 876044).
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Comment on attachment 8470014 [details] [diff] [review]
> sharing indicator in main menu for keyboard accessibility,
> 
> Review of attachment 8470014 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/locales/en-US/chrome/browser/browser.properties
> @@ +536,5 @@
> > +getUserMedia.sharingMenu.label = Tabs sharing devices
> > +getUserMedia.sharingMenu.accesskey = d
> > +getUserMedia.sharingMenuCamera = %S (camera)
> > +getUserMedia.sharingMenuMicrophone = %S (microphone)
> > +getUserMedia.sharingMenuScreen = %S (screen)
> 
> There should be localization notes above each of these describing what the
> %S will be replaced with.

We can do a single note that applies to all of these. Will do.

> ::: browser/modules/webrtcUI.jsm
> @@ +581,5 @@
> > +      tabIdentifier = streamInfo.browser.currentURI.host;
> > +    } catch (ex) {};
> > +    if (!tabIdentifier) {
> > +      // This is unfortunate, but we should display *something*...
> > +      tabIdentifier = streamInfo.browser.currentURI.scheme + ":...";
> 
> So this could potentially show "https:..." ? I don't see that as being
> useful. We should probably have some defined fallback text that says "(host
> unknown, proceed with caution)"

No, host only throws for things like data:... which don't have a host. As for "proceed with caution", that's interesting, but at this point you're already sharing stuff, and clicking these items will just focus the tab and/or drop down the doorhanger, so I don't think we need a different string for this.

(In reply to Florian Quèze [:florian] [:flo] from comment #17)
> Comment on attachment 8470014 [details] [diff] [review]
> sharing indicator in main menu for keyboard accessibility,
> 
> Review of attachment 8470014 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/modules/webrtcUI.jsm
> @@ +577,5 @@
> > +
> > +    let tabIdentifier;
> > +    try {
> > +      // This can throw for some types of URLs.
> > +      tabIdentifier = streamInfo.browser.currentURI.host;
> 
> This will show the hostname of the tab's top level document rather than the
> host actually using the device (which could be a different one if there are
> frames involved).
> 
> We used to have this behavior, until it was changed in a security bug (bug
> 876044).

I'll look at this again. I guess I should also audit my use of the content documents to get browsers, as they might not be the top level document and therefore getBrowserForDocument might fail...
(In reply to :Gijs Kruitbosch from comment #18)

> > ::: browser/modules/webrtcUI.jsm
> > @@ +581,5 @@
> > > +      tabIdentifier = streamInfo.browser.currentURI.host;
> > > +    } catch (ex) {};
> > > +    if (!tabIdentifier) {
> > > +      // This is unfortunate, but we should display *something*...
> > > +      tabIdentifier = streamInfo.browser.currentURI.scheme + ":...";
> > 
> > So this could potentially show "https:..." ? I don't see that as being
> > useful. We should probably have some defined fallback text that says "(host
> > unknown, proceed with caution)"
> 
> No, host only throws for things like data:... which don't have a host.

The likely URL without host is "about:loopconversation". I wonder if it needs special casing.
I believe this is otherwise fine. I've updated this to use asciiHost, and to use the 'right' URL, with a case that should handle about: pages, and a fallback to 'Unknown origin' for other protocols.
Attachment #8471493 - Flags: review?(florian)
Attachment #8470014 - Attachment is obsolete: true
Forgot to actually test the file: case... had to move the bundle declaration about a bit. Also, just to clarify, I'm asking florian for review this time because he's back from PTO and because gUM is, to a certain degree, 'his baby' ;-).
Attachment #8471496 - Flags: review?(florian)
Attachment #8471493 - Attachment is obsolete: true
Attachment #8471493 - Flags: review?(florian)
Comment on attachment 8471496 [details] [diff] [review]
sharing indicator in main menu for keyboard accessibility,

Review of attachment 8471496 [details] [diff] [review]:
-----------------------------------------------------------------

I think you could easily add some testing here:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/head.js#518
Check that the menu is hidden/not hidden. And maybe check that the menuitem count is 0 or > 0.

::: browser/modules/webrtcUI.jsm
@@ +628,5 @@
> +  let document = aWindow.document;
> +  let menu = document.getElementById("tabSharingMenu");
> +  if (!menu) {
> +    let stringBundle = aWindow.gNavigatorBundle;
> +    menu = document.createElement("menu");

Is there a reason for creating this menu dynamically rather than just having it in the xul file and hiding/showing it when needed?

@@ +694,5 @@
>    }
>  
> +  let browserWindowEnum = Services.wm.getEnumerator("navigator:browser");
> +  while (browserWindowEnum.hasMoreElements()) {
> +    let chromeWin = browserWindowEnum.getNext();

Would it be a useful optimization to skip here the popup windows that are never going to have a visible menubar? I guess it isn't, but I figured I would mention it anyway so that you can think about it too.
(In reply to Florian Quèze [:florian] [:flo] from comment #22)
> Comment on attachment 8471496 [details] [diff] [review]
> sharing indicator in main menu for keyboard accessibility,
> 
> Review of attachment 8471496 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think you could easily add some testing here:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/
> general/head.js#518
> Check that the menu is hidden/not hidden. And maybe check that the menuitem
> count is 0 or > 0.

I can look into it.

> ::: browser/modules/webrtcUI.jsm
> @@ +628,5 @@
> > +  let document = aWindow.document;
> > +  let menu = document.getElementById("tabSharingMenu");
> > +  if (!menu) {
> > +    let stringBundle = aWindow.gNavigatorBundle;
> > +    menu = document.createElement("menu");
> 
> Is there a reason for creating this menu dynamically rather than just having
> it in the xul file and hiding/showing it when needed?

Yes. The menu on Mac comes from http://mxr.mozilla.org/mozilla-central/source/toolkit/content/macWindowMenu.inc and we shouldn't introduce this menu in toolkit.

> @@ +694,5 @@
> >    }
> >  
> > +  let browserWindowEnum = Services.wm.getEnumerator("navigator:browser");
> > +  while (browserWindowEnum.hasMoreElements()) {
> > +    let chromeWin = browserWindowEnum.getNext();
> 
> Would it be a useful optimization to skip here the popup windows that are
> never going to have a visible menubar? I guess it isn't, but I figured I
> would mention it anyway so that you can think about it too.

This code should be fairly quick anyway - there are unlikely to be more than 10-20 windows, so this isn't really a hot loop. Plus, on OSX "even" popup menus have a menu. So no, I don't think it's worth the added complexity.
Now with a test addition. Not sure how to test the number of menuitems easily as that depends on popupshowing, so it'd involve interacting with the menus, which is different cross-platform and so on... I'd rather we do that as part of bug 1041658.
Attachment #8471902 - Flags: review?(florian)
Attachment #8471496 - Attachment is obsolete: true
Attachment #8471496 - Flags: review?(florian)
Comment on attachment 8471902 [details] [diff] [review]
sharing indicator in main menu for keyboard accessibility,

Review of attachment 8471902 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, assuming the test is green.

::: browser/base/content/test/general/head.js
@@ +550,4 @@
>    is(ui.showGlobalIndicator, expected, msg);
> +  let windows = Services.wm.getEnumerator("navigator:browser");
> +  let expectedChildren = expected ? "at least 1 child" : "no children";
> +  expectedChildren = "Should have " + expectedChildren;

Please remove these 2 lines of dead code.
Attachment #8471902 - Flags: review?(florian) → review+
(In reply to Florian Quèze [:florian] [:flo] from comment #25)
> Comment on attachment 8471902 [details] [diff] [review]
> sharing indicator in main menu for keyboard accessibility,
> 
> Review of attachment 8471902 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me, assuming the test is green.
> 
> ::: browser/base/content/test/general/head.js
> @@ +550,4 @@
> >    is(ui.showGlobalIndicator, expected, msg);
> > +  let windows = Services.wm.getEnumerator("navigator:browser");
> > +  let expectedChildren = expected ? "at least 1 child" : "no children";
> > +  expectedChildren = "Should have " + expectedChildren;
> 
> Please remove these 2 lines of dead code.

Pushed to try with that:

Results will be displayed on TBPL as they come in:
https://tbpl.mozilla.org/?tree=Try&rev=e84e5f98e207

Alternatively, view them on Treeherder (experimental):
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e84e5f98e207
remote:   https://hg.mozilla.org/integration/fx-team/rev/3f0da78fc8e1
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/3f0da78fc8e1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
Iteration: 34.2 → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
So this is where the "Tabs sharing devices" came from...

Works on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140820030202 CSet: ffdd1a398105

Can somebody please take a look at bug 1056179 as the non keyboard version of the indicator is not quite working though?
Status: RESOLVED → VERIFIED
Whiteboard: [bugday-20140820]
Depends on: 1064909
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: