Closed Bug 1027401 Opened 10 years ago Closed 10 years ago

Orientation lock is not restored after popup is closed

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: mclemmons, Assigned: alive)

References

Details

(Keywords: branch-patch-needed, regression, Whiteboard: [2.0-flame-test-run-2] [p=2])

Attachments

(3 files)

Attached image Screenshot of bunched together text (deleted) —
User taps Contacts App, selects the gear icon and import Contacts button. After switching to landscape and selecting Gmail and logging in with valid credentials, the Found x friends to import screen displays the A-# list in the right margin in a manner that is difficult to read as the characters are bunched together. This is not the case in portrait mode. Prerequisites: 1. Have Wi-Fi and/or Data Connection enabled 2. Have active Gmail account Repro Steps: 1) Update a Flame to 20140618000202 2) Tap Contacts App and select gear icon in upper right 3) Select Import contacts and change the orientation of the device to landscape 4) Select Gmail and log in with valid credentials 5) Observe device behavior Actual: Found x friends to import screen displays the A-# list in the right margin in a manner that is difficult to read as the characters are bunched together. This is not the case in portrait mode. Expected: A-# list of characters are spaced enough to be readable, similar to portrait mode. Repro frequency: (10/10, 100%, etc.) Link to failed test case: None. This was found during exploratory testing around an existing test case. See attached: a) screenshot b) Flame logcat unavailable due to https://bugzilla.mozilla.org/show_bug.cgi?id=1010993 c) Open C logcat provided instead of Flame logcat described in b) above Environmental Variables: Device: Flame 2.0 Build ID: 20140618000202 Gaia: 83844c7679b3b9f6e7f1116c1eeec2d1e7a64eec Gecko: 55679dc2e72b Version: 32.0a2 (2.0) Firmware Version: v121-2 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
This issue DOES reproduce on Flame 2.1, Buri2.1, Open C 2.1, Flame 2.0, Buri 2.0, and Open C 2.0 Flame 2.1 Environmental Variables: Device: Flame Master Build ID: 20140618040513 Gaia: 431aed0a7c7560c6eacd35ea69aa0a7a4ebe72c7 Gecko: 37f08ddaea48 Version: 33.0a1 (Master) Firmware Version: v121-2 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Open_C 2.1 Environmental Variables: Device: Open_C Master Build ID: 20140618040513 Gaia: 431aed0a7c7560c6eacd35ea69aa0a7a4ebe72c7 Gecko: 37f08ddaea48 Version: 33.0a1 (Master) Firmware Version: P821A10V1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Buri 2.1 Environmental Variables: Device: Buri Master Build ID: 20140618073003 Gaia: 336c30b6147cdd9122ad0b2bbffb81eb869a9ec2 Gecko: 1cea544c74c5 Version: 33.0a1 (Master) MOZ Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Buri 2.0 Environmental Variables: Device: Buri 2.0 Build ID: 20140618063014 Gaia: 83844c7679b3b9f6e7f1116c1eeec2d1e7a64eec Gecko: 883d156210cf Version: 32.0a2 (2.0) MOZ Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Open_C 2.0 Environmental Variables: Device: Open_C 2.0 Build ID: 20140618000202 Gaia: 83844c7679b3b9f6e7f1116c1eeec2d1e7a64eec Gecko: 55679dc2e72b Version: 32.0a2 (2.0) Firmware Version: P821A10V1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 The Alphabet Picker on the right side is squished together and hard to read when in landscape orientation _________________________________________________________________________________________ This issue does NOT reproduce on Flame 1.4, Buri 1.4 or Open C 1.4 Buri 1.4 Environmental Variables: Device: Buri 1.4 Build ID: 20140618063004 Gaia: fc74015d26bcbc3e31a45d34cb65777112a35982 Gecko: fab72d8aa2e0 Version: 30.0 (1.4) MOZ Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 Flame 1.4 Environmental Variables: Device: Flame 1.4 Build ID: 20140618000203 Gaia: 3bdd037ec1a11abebe16a5d7f6ff0d863e80bc07 Gecko: 523491fa3339 Version: 30.0 (1.4) Firmware Version: v121-2 User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 Open_C 1.4 Environmental Variables: Device: Open_C 1.4 Build ID: 20140618000203 Gaia: 3bdd037ec1a11abebe16a5d7f6ff0d863e80bc07 Gecko: 523491fa3339 Version: 30.0 (1.4) Firmware Version: P821A10V1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 1.4 Devices do not change to landscape orientation when turning the device horizontally and accessing gmail
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Attached file Landscape view issue with Imports (deleted) —
The actual issue here is that the user can go into landscape mode. This should be locked to portrait mode. Please change the title of the issue to reflect this. Also, the regression and regressionwindow-wanted tags should be added here since this is a regression.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(mclemmons)
Flags: needinfo?(mclemmons) → needinfo?(ktucker)
Summary: B2G][Contacts][Gmail]Import Contacts while in landscape view displays letters along right margin bunched together → [B2G][Contacts]Importing contacts from Gmail login screen is allowed to display in landscape mode rather than remain locked in portrait mode
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Kevin - This is missing a blocking triage analysis on the bug.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage+][lead-review-] → [QAnalyst-Triage?][lead-review-]
QA Whiteboard: [QAnalyst-Triage?][lead-review-]
Nominating 2.0? since this is a regression from 1.4. This issue could lead to a more severe bug since the contacts app orientation is no longer locked in portrait mode.
blocking-b2g: --- → 2.0?
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage+][lead-review+]
blocking-b2g: 2.0? → 2.0+
QA Contact: pcheng
QA Whiteboard: [QAnalyst-Triage+][lead-review+]
Target Milestone: --- → 2.0 S5 (4july)
Whiteboard: [2.0-flame-test-run-2] → [2.0-flame-test-run-2] [p=2]
Assignee: nobody → francisco
b2g-inbound Regression Window: Last Working Environmental Variables: Device: Flame Build ID: 20140424023002 Gaia: 4a4a078768423426c7c0fc580c8642bc735033a5 Gecko: 91946180b1e7 Version: 31.0a1 (Master) Firmware Version: v122 First Broken Environmental Variables: Device: Flame Build ID: 20140424053001 Gaia: 4b6c9d4929c57005ab142e8eb3dc6783d55c427c Gecko: c30df002e626 Version: 31.0a1 (Master) Firmware Version: v122 Last Working Gaia / First Broken Gecko: Issue Does NOT reproduce Gaia: 4a4a078768423426c7c0fc580c8642bc735033a5 Gecko: c30df002e626 Last Working Gecko / First Broken Gaia: Issue DOES reproduce Gaia: 4b6c9d4929c57005ab142e8eb3dc6783d55c427c Gecko: 91946180b1e7 Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/4a4a078768423426c7c0fc580c8642bc735033a5...4b6c9d4929c57005ab142e8eb3dc6783d55c427c
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Hi, I've been checking this and code in contacts didn't change at all. Looking at comment 6 for the regression window, there was a change in the window manager affecting the popupmanager. That contacts list is being opened in a window.open, so I guess the popupmanager within the window manager is not honoring the orientation in the manifest for the app. Alive should this be a system app bug? Or we should modify the orientation in the manifest. Thanks!
Flags: needinfo?(alive)
bug 916709 broke this.
Blocks: popup-window
Component: Gaia::Contacts → Gaia::System::Window Mgmt
Interesting. I and Francisco had some discussion on IRC and AFAIK we don't have perfect solution for this kind of issues. Sometimes the opened page don't want to be orientation-free and sometimes it would like to. Yes, we could lock the orientation according to the app opens the page, but I'd like not to block on this issue because I am afraid one day there would be another bug talking about: "why the page opened by window.open cannot be rotated?"
Flags: needinfo?(alive)
More clearly, I don't think this is a blocker - unless 1. User cannot import contact when stay landscape mode 2. When user finish/cancels the import and back to contact app, the contact app becomes orientation-free.
I do agree with :alive here, not a real blocker unless we cannot import contacts. Is not a trivial solution, since we could break other stuff, it will require several investigation and sharing with people to be aware of possible new behavior. Can we unblock?
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #9) > I and Francisco had some discussion on IRC and AFAIK we don't have perfect > solution for this kind of issues. Sometimes the opened page don't want to be > orientation-free and sometimes it would like to. > > Yes, we could lock the orientation according to the app opens the page, > but I'd like not to block on this issue because I am afraid one day there > would be another bug talking about: "why the page opened by window.open > cannot be rotated?" Let's WONTFIX this bug. Sounds to me that having the popup window rotatable is a new feature and this bug is literally describing that as a regression.
Status: NEW → RESOLVED
blocking-b2g: 2.0+ → ---
Closed: 10 years ago
Resolution: --- → WONTFIX
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12) > (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #9) > > I and Francisco had some discussion on IRC and AFAIK we don't have perfect > > solution for this kind of issues. Sometimes the opened page don't want to be > > orientation-free and sometimes it would like to. > > > > Yes, we could lock the orientation according to the app opens the page, > > but I'd like not to block on this issue because I am afraid one day there > > would be another bug talking about: "why the page opened by window.open > > cannot be rotated?" > > Let's WONTFIX this bug. Sounds to me that having the popup window rotatable > is a new feature and this bug is literally describing that as a regression. No. Visual refresh is a critical requirement for 2.0. Anything that involves bunching up a bunch of letters to the point they are not readable is not something we can ship with. Reopening.
Status: RESOLVED → REOPENED
blocking-b2g: --- → 2.0+
Resolution: WONTFIX → ---
Let's confirm with UX on whether or not comment 12 is a desired behavior change or not. (In reply to Jason Smith [:jsmith] from comment #13) > No. Visual refresh is a critical requirement for 2.0. Anything that involves > bunching up a bunch of letters to the point they are not readable is not > something we can ship with. Reopening. I don't understand what is not readable in this comment...
Flags: needinfo?(firefoxos-ux-bugzilla)
Hi folks, I think that visually the problem is with the shortcut letters on the right, used to jump from letter to letter. I was about to suggest a solution in contacts, based on media queries we just hide this control if we are in landscape and don't have space to show this. But testing it, I realise about a bigger problem, once you one a window a go to landscape mode, if you close the window in landscape mode, the opener app, goes to landscape mode, which to me is a blocker. Just recorded a video: https://drive.google.com/file/d/0B3RfQh7fuZjAaWpoYVlXc2hnSkU/edit?usp=sharing As per conversation with Alive, I'm unassigning myself, since I guess he would like to take over this. Thanks! F.
Assignee: francisco → nobody
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #15) > Hi folks, > > I think that visually the problem is with the shortcut letters on the right, > used to jump from letter to letter. > > I was about to suggest a solution in contacts, based on media queries we > just hide this control if we are in landscape and don't have space to show > this. > > But testing it, I realise about a bigger problem, once you one a window a go > to landscape mode, if you close the window in landscape mode, the opener > app, goes to landscape mode, which to me is a blocker. > > Just recorded a video: > > https://drive.google.com/file/d/0B3RfQh7fuZjAaWpoYVlXc2hnSkU/edit?usp=sharing > > As per conversation with Alive, I'm unassigning myself, since I guess he > would like to take over this. > > Thanks! > F. This should be fixed but I am still questioning the opened window should be orientation free. Maybe we need to open another bug to fix the regression you found. Waiting UX decision.
Pinging Carrie, UX owner for contacts. Carrie, do you think that an acceptable solution could be hide the shortcuts on the right if we detect that we don't have space to display them? So we could fix this bug, and as Alive comments, open another one to solve the problem in comment 15
Flags: needinfo?(cawang)
Carrie is already flagged so removing general UX flag.
Flags: needinfo?(firefoxos-ux-bugzilla)
My suggestion would be, if we are going to provide the landscape in this case, we should have a proper alphabet picker for users since the space to show the list is really really small, users may rely on this indicator very much. However, I'd go with comment 16 that we shouldn't provide the orientation-free opened window in Contacts. Because the Contacts APP doesn't have landscape mode, I think we shall keep it consistent. Can we apply this solution? If not, then we'll need VD to provide visual spec for this landscape alphabet picker. Thanks!
Flags: needinfo?(cawang) → needinfo?(francisco)
(In reply to Carrie Wang [:carrie] from comment #19) > My suggestion would be, if we are going to provide the landscape in this > case, we should have a proper alphabet picker for users since the space to > show the list is really really small, users may rely on this indicator very > much. > However, I'd go with comment 16 that we shouldn't provide the > orientation-free opened window in Contacts. Because the Contacts APP doesn't > have landscape mode, I think we shall keep it consistent. Can we apply this > solution? If not, then we'll need VD to provide visual spec for this > landscape alphabet picker. Thanks! Carrie, Alive is looking at a solution for not displaying the landscape. Don't know if we'll manage to have it or not, but in case we are in landscape finally for that screen, IMHO, the alphabet will be unusable even if we properly display it, since it will be super small, I'm sure we'll end up touching several letters cause of the space.
Flags: needinfo?(francisco)
Hmmm...wait, do I misunderstand? Isn't the page is under google? What's different if we are using browser app to import google contact? Should we block orientation in this case? How do we know we are opening a web page which is not orientation-free-friendly?
Right the page is free orientation friendly, the problem is that we are keeping the orientation to the app, but that's a different thing, right? Should we close this, and keep just the one (don't have the bug number), for the orientation changing after a opened window is changing orientation?
After inspecting System app with App Manager, Alive found that the screenshot depicted in comment 0 is NOT the popup, but the import page in Contact iframe after the pop-up is closed. This is indeed a valid regression, not a special use case. Sorry for the confusion everyone.
Assignee: nobody → alive
Status: REOPENED → ASSIGNED
Summary: [B2G][Contacts]Importing contacts from Gmail login screen is allowed to display in landscape mode rather than remain locked in portrait mode → Orientation lock is not restored after popup is closed
Root cause https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L1819 doesn't work. The popupclosing is dispatched in the popup element due to bug 1024168, but in AppWindow.prototype.handleEvent, it's stopping the propagation of all events of the front window. This should be corrected.
Attached file patch for master, v1 (deleted) —
Change event handling mechanism again: * Do not propagate any mozbrowser events unless we have rearWindow * Do removing rear/front/next/previous only on destroyed * Only care about child's event
Attachment #8451546 - Flags: review?(etienne)
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #25) > Created attachment 8451546 [details] > patch for master, v1 > > Change event handling mechanism again: > * Do not propagate any mozbrowser events unless we have rearWindow > * Do removing rear/front/next/previous only on destroyed > * Only care about child's event The way injecting popupclosing/activityclosing to each window seems stupid, maybe we should do it in the child_window_factory.
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #26) > (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #25) > > Created attachment 8451546 [details] > > patch for master, v1 > > > > Change event handling mechanism again: > > * Do not propagate any mozbrowser events unless we have rearWindow > > * Do removing rear/front/next/previous only on destroyed > > * Only care about child's event > > The way injecting popupclosing/activityclosing to each window seems stupid, > maybe we should do it in the child_window_factory. But a little out of scope so I plan to do it in another bug.
Comment on attachment 8451546 [details] patch for master, v1 v2: I end up moving closing handler into child window to reduce duplicate work. Also it prevents popup is listening to its own popupclosing event.
Comment on attachment 8451546 [details] patch for master, v1 Makes sense! (tiny nit on github)
Attachment #8451546 - Flags: review?(etienne) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Looking..
Flags: needinfo?(alive)
Strange, the same patch works for me locally.
Very odd, it was perma-failing on Aurora yesterday until the revert landed.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #37) > Very odd, it was perma-failing on Aurora yesterday until the revert landed. I am going to merge again, please backout if still happens due to the merge. https://github.com/mozilla-b2g/gaia/commit/18c44a1bc31b374ba00a069904465a8d07971a60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: