Closed
Bug 1059220
Opened 10 years ago
Closed 10 years ago
display window count of each application in applications list of the app share prompt
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
People
(Reporter: florian, Assigned: florian)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
The mock-up in attachment 8475862 [details] from bug 1053221 showed the window count for each application.
This wasn't implemented as part of bug 1057006 because it required additional platform support, which is being added in bug 1058766.
Flags: qe-verify+
Flags: firefox-backlog+
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8479809 -
Flags: review?(gijskruitbosch+bugs)
Comment 2•10 years ago
|
||
Comment on attachment 8479809 [details] [diff] [review]
Patch
Review of attachment 8479809 [details] [diff] [review]:
-----------------------------------------------------------------
Curiosity question: what is shared when sharing an app with 0 windows on OS X?
Are we happy just showing this in the prompt and not in other places? :-)
::: browser/modules/webrtcUI.jsm
@@ +360,5 @@
> name = devices[i].name;
> + if (type == "application") {
> + // The application names returned by the platform are of the form:
> + // <window count>\x1e<application name>
> + let sepIndex = name.indexOf("\x1e");
Holy hackman batman. Why not just create a separate field on the device object? :-\
Attachment #8479809 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #2)
> Curiosity question: what is shared when sharing an app with 0 windows on OS
> X?
I wondered the same thing. We share a completely blank stream, and once a window of the application is displayed, it appears correctly.
> Are we happy just showing this in the prompt and not in other places? :-)
Not really, but we aren't more unhappy about this than about not being able to tell in the other indicators what window/application is being shared. I'm hoping all of this will be fixed correctly by the redesign for 35.
> ::: browser/modules/webrtcUI.jsm
> @@ +360,5 @@
> > name = devices[i].name;
> > + if (type == "application") {
> > + // The application names returned by the platform are of the form:
> > + // <window count>\x1e<application name>
> > + let sepIndex = name.indexOf("\x1e");
>
> Holy hackman batman. Why not just create a separate field on the device
> object? :-\
Short answer is that the webrtc folks didn't want to change the device interface for 34.
Longer answer can be found in #media IRC logs from last Thursday I think:
22:02:26 - jesup: linuxwolf: I presume adding a number of windows to the descriptions when getting the list wouldn't be too hard?
22:02:33 - linuxwolf: hrm
22:02:58 - flo-retina: jesup: note the plural string.
22:03:10 - linuxwolf: I don't think so, but it'll take some doing for localization down to that layer of the code
22:04:09 - linuxwolf: or tweaking of a number of interfaces to bubble it up separately
22:04:09 - flo-retina: maybe we could find a way to expose the screen count to the UI, and have the UI deal with the localized plural string?
22:05:14 - linuxwolf: if it's ok for the display name to have semantic meaning (e.g., <window count> <app name>"), then it's very doable
22:05:41 - linuxwolf: (or whatever the property is called at the UI layer)
22:06:55 - flo-retina: hmm, that's a bit strange, but seems like an easy solution :).
22:07:20 - linuxwolf: certainly easier than changing a bunch of interfaces and their impls (-:
22:08:21 - jesup: flo-retina: I'm good with linuxwolfs, or just application names, No major reworks please (and not localization nightmares ;-)
22:10:04 - flo-retina: jesup: alright, let's parse the display name in the UI then :)
Assignee | ||
Comment 4•10 years ago
|
||
Updated•10 years ago
|
Iteration: --- → 34.3
Florin, we don't have a feature owner for app sharing yet. Is this something your team can take on ? I will bring it up with Marc as well. Thanks!
QA Whiteboard: appsharing
Flags: needinfo?(florin.mezei)
QA Contact: florin.mezei
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment 7•10 years ago
|
||
(In reply to Liz Henry :lizzard from comment #5)
> Florin, we don't have a feature owner for app sharing yet. Is this something
> your team can take on ? I will bring it up with Marc as well. Thanks!
Appsharing is pretty much another form of screensharing, so I'm guessing this feature should go to Nils. Liz, can you talk to Nils and see if he should take these app sharing bugs and send me an email? I've had a look today over app sharing, so if needed I can continue working on bugs related to it.
With regards to this bug, I've verified it today on the latest Firefox Nightly 34 (BuildID=20140828030205) on Windows 7 x64, Mac OS X 10.8.5 and Ubuntu 13.04 x64, and I can see the window count displayed for each item in the application list, whether it's 1, 2 or more windows.
I see some issues with the actual counting (e.g. count does not update unless Firefox is restarted, counting of some applications does not look right on some OS), but these seem to belong under bug 1058766.
The steps used to test this were:
- had multiple applications with multiple windows open
- started Firefox Nightly 34
- ensured media.getusermedia.screensharing.enabled = true
- set media.getusermedia.screensharing.allowed_domains = queze.net
- opened http://queze.net/goinfre/ff_gum_test.html
- started Application
- clicked on the "Application to share:" drop-down to get the list of applications
Let me know if you think anything more should be done for this.
Status: RESOLVED → VERIFIED
Flags: needinfo?(florin.mezei) → needinfo?(lhenry)
QA Contact: florin.mezei → drno
Clearing needinfo a bit late.... thanks Florin!
Flags: needinfo?(lhenry)
You need to log in
before you can comment on or make changes to this bug.
Description
•