Closed Bug 1079761 Opened 5 years ago Closed 5 years ago

add 'stop tab mirroring' one level higher in the menu

Categories

(Firefox for Android :: Screencasting, defect)

35 Branch
All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 36
Tracking Status
firefox33 --- wontfix
firefox34 --- verified
firefox35 --- verified
firefox36 --- verified
fennec 34+ ---

People

(Reporter: krudnitski, Assigned: liuche)

References

Details

Attachments

(1 file, 2 obsolete files)

Just like we do with guest browsing, can we:

- keep the function to start tab mirroring in the menu - tools menu, BUT
- bring the function to stop up one level higher in the menu (so you don't need to go back into tools to stop it)
tracking-fennec: ? → 34+
Chenxia - Simple bug. Just move the menu.
Assignee: nobody → liuche
Attached patch Patch: tab mirroring (obsolete) — Splinter Review
I can't actually test this because I don't have a roku/chromecast to send things to. Here's an apk:

http://people.mozilla.org/~liuche/bug-1072831/tab-mirror.apk
Attachment #8502754 - Flags: review?(mark.finkle)
Wes, let me know if you managed to get casting working, and if you can see if this patch/build is working though rbarker mentioned that chromecast might need bug 1080012, bug 1048335, and bug 1080701 to work now, which is not in my test build's patch queue.
Flags: needinfo?(wjohnston)
Comment on attachment 8502754 [details] [diff] [review]
Patch: tab mirroring

Randall sorta owns this now
Attachment #8502754 - Flags: review?(mark.finkle) → review?(rbarker)
Flags: needinfo?(wjohnston)
Comment on attachment 8502754 [details] [diff] [review]
Patch: tab mirroring

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

::: mobile/android/chrome/content/CastingApps.js
@@ +135,5 @@
>            }
>            NativeWindow.menu.update(this.mirrorStartMenuId, { visible: true });
>            NativeWindow.menu.update(this.mirrorStopMenuId, { visible: false });
>          }.bind(this),
> +        parent: NativeWindow.menu

This didn't work for me, the "stop mirror" option did not get inserted into any menu. :wesj suggested just removing this line and that seems to have the desired effect.
Attached patch Patch: tab mirroring v2 (obsolete) — Splinter Review
Updated patch.
Attachment #8502754 - Attachment is obsolete: true
Attachment #8502754 - Flags: review?(rbarker)
Attachment #8504203 - Flags: review+
Thanks, Randall! Landed:

https://hg.mozilla.org/integration/fx-team/rev/b072d6d6d730
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 36
Comment on attachment 8504203 [details] [diff] [review]
Patch: tab mirroring v2

Approval Request Comment
[Feature/regressing bug #]: Make exiting Guest mode easier
[User impact if declined]: To exit guest mode from menu, user needs to go down two menus, instead of just one
[Describe test coverage new/current, TBPL]: local testing
[Risks and why]: very very low, one-line change in deleting hierarchy
[String/UUID change made/needed]: none
Attachment #8504203 - Flags: approval-mozilla-aurora?
Comment on attachment 8504203 [details] [diff] [review]
Patch: tab mirroring v2

Derp, I meant tab casting, not guest session.

Approval Request Comment
[Feature/regressing bug #]: Make exiting Tab Mirroring easier
[User impact if declined]: To stop sharing a tab, user needs to go down two menus, instead of just one
[Describe test coverage new/current, TBPL]: local testing
[Risks and why]: very very low, one-line change in deleting hierarchy
[String/UUID change made/needed]: none
This landed with the wrong bug number in the commit message. Please double-check that in the future. :)
https://hg.mozilla.org/mozilla-central/rev/b072d6d6d730
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8504203 [details] [diff] [review]
Patch: tab mirroring v2

Approval Request Comment
[Feature/regressing bug #]: Make exiting Tab Mirroring easier
[User impact if declined]: To stop sharing a tab, user needs to go down two menus, instead of just one
[Describe test coverage new/current, TBPL]: local testing, fx-team green
[Risks and why]: very very low, one-line change in deleting hierarchy
[String/UUID change made/needed]: none
Attachment #8504203 - Flags: approval-mozilla-beta?
Updating patch message because it had the wrong bug number. Carrying over r+.

Approval Request Comment
[Feature/regressing bug #]: Make exiting Tab Mirroring easier
[User impact if declined]: To stop sharing a tab, user needs to go down two menus, instead of just one
[Describe test coverage new/current, TBPL]: local testing, fx-team green
[Risks and why]: very very low, one-line change in deleting hierarchy
[String/UUID change made/needed]: none
Attachment #8504203 - Attachment is obsolete: true
Attachment #8504203 - Flags: approval-mozilla-beta?
Attachment #8504203 - Flags: approval-mozilla-aurora?
Attachment #8505696 - Flags: review+
Attachment #8505696 - Flags: approval-mozilla-beta?
Attachment #8505696 - Flags: approval-mozilla-aurora?
Comment on attachment 8505696 [details] [diff] [review]
Patch: tab mirroring

Beta+
Aurora+
Attachment #8505696 - Flags: approval-mozilla-beta?
Attachment #8505696 - Flags: approval-mozilla-beta+
Attachment #8505696 - Flags: approval-mozilla-aurora?
Attachment #8505696 - Flags: approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
I've updated both SUMO articles to reflect the new location: https://support.mozilla.org/kb/send-webpages-your-tv-firefox-android-chromecast and https://support.mozilla.org/kb/view-webpages-on-tv-roku-and-firefox-android

Did the label used to say "Stop mirroring" or am I mistaken? I think it sounds better than "Stop mirror" but the meaning's clear either way.
Verified as fixed in builds:
- 36.0a1 2014-11-25;
- 35.0a2 2014-11-25;
- 34 RC1;
Device: Nexus 4 (Android 4.4.4).
You need to log in before you can comment on or make changes to this bug.