Closed
Bug 701771
Opened 11 years ago
Closed 11 years ago
Context Click menu shows FS option even within FS mode
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: tchung, Assigned: Margaret)
References
()
Details
(Whiteboard: [testday-20111111])
Attachments
(2 files)
Log into full screen video mode, and notice that context click menu still gives the option for full screen. See screenshot Repro: 1) install fennec nightly, 20111111 build 2) visit a site with full screen video (see URL) 3) context click the video sample at the top, and enter full screen 4) once in FS, context click again, and verify the option is still available Expected: - FS mode should hide the FS context click option
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → margaret.leibovic
Assignee | ||
Updated•11 years ago
|
Summary: Context Click menu shows FS option even within FS movde → Context Click menu shows FS option even within FS mode
Assignee | ||
Comment 1•11 years ago
|
||
We'll have to add a new selector when we implement the controls in bug 701785 since we want them to show up regardless of full screen state, but I think this fix makes sense for now since this is the only video context menu item we have right now.
Attachment #573933 -
Flags: review?(wjohnston)
Comment 2•11 years ago
|
||
Comment on attachment 573933 [details] [diff] [review] patch LGTM
Attachment #573933 -
Flags: feedback+
Comment 3•11 years ago
|
||
Comment on attachment 573933 [details] [diff] [review] patch Review of attachment 573933 [details] [diff] [review]: ----------------------------------------------------------------- I think I'd rather we actually just not cache this Selector. i.e. My original plan was basically to only cache things that I thought we or extensions might need easy access too for more than a few menuitems. That is a fairly vague and nebulous set, but for this one, can we just do something like: this.add(Strings.browser.GetStringFromName("contextmenu.fullScreen"), this.SelectorContext("video:not(:-moz-full-screen)"), function(aTarget) { aTarget.mozRequestFullScreen(); } );
Attachment #573933 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #3) > Comment on attachment 573933 [details] [diff] [review] [diff] [details] [review] > patch > > Review of attachment 573933 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > I think I'd rather we actually just not cache this Selector. i.e. My > original plan was basically to only cache things that I thought we or > extensions might need easy access too for more than a few menuitems. That is > a fairly vague and nebulous set, but for this one, can we just do something > like: > > this.add(Strings.browser.GetStringFromName("contextmenu.fullScreen"), > this.SelectorContext("video:not(:-moz-full-screen)"), > function(aTarget) { > aTarget.mozRequestFullScreen(); > } > ); That makes sense - I was just following the model of the other Selectors :)
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/a28cb9b7b7f4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 6•11 years ago
|
||
20111114041052 http://hg.mozilla.org/projects/birch/rev/859ecdfe0168 Samsung Galaxy SII (Android 2.3.4)
Status: RESOLVED → VERIFIED
Updated•1 year ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•