change build-in apps orientation to 'default'

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gasolin, Assigned: gasolin)

Tracking

unspecified
Other
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(b2g-v1.2 wontfix)

Details

(Whiteboard: [Flatfish only])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
In current design, the homescreen/email/calendar are fixed fixed in portrait mode in phone. Will fixed in landscape mode in tablet. 

So there will be a new orientation mode called 'default' from gecko, which will specify the default orientation mode. And present the correspondent mode per device. (will do that in another patch after that feature is implemented)
(Assignee)

Updated

5 years ago
Blocks: 903304
blocking-b2g: --- → koi?
(Assignee)

Updated

5 years ago
Summary: [homescreen][email][calendar] change orientation to 'default' → [homescreen][email][calendar][communication] change orientation to 'default'
Viral, do we have bug filed for Gecko implementation? Thanks.
Flags: needinfo?(vwang)

Comment 2

5 years ago
Yes, the bug 908058 is used for tracking the gecko part.
Depends on: 908058
Flags: needinfo?(vwang)

Updated

5 years ago
blocking-b2g: koi? → koi+
(Assignee)

Comment 3

5 years ago
update screen orientation from 'portrait-primary' to 'default'
https://github.com/mozilla-b2g/gaia/pull/12016

bluetooth, calendar, camera, clock, communication, costcontrol, email, fm, homescreen, music, pdf.js,
settings, sms, wallpaper, wappush, system

can't test in nightly or emulator since 'default' orientation is not ready yet.
Summary: [homescreen][email][calendar][communication] change orientation to 'default' → [homescreen] change build-in apps orientation to 'default'
(Assignee)

Updated

5 years ago
Assignee: nobody → gasolin
Summary: [homescreen] change build-in apps orientation to 'default' → change build-in apps orientation to 'default'
(Assignee)

Comment 4

5 years ago
Created attachment 801346 [details]
pull request redirect to github

@tim, I'm not sure if we need to change system's attention_screen/lock_screen/cards_view default orientation? 
Since they works fine in current values.
Attachment #801346 - Flags: feedback?(timdream)
(Assignee)

Updated

5 years ago
Attachment #801346 - Flags: review?
Attachment #801346 - Flags: feedback?(alive)
(Assignee)

Updated

5 years ago
Attachment #801346 - Flags: review?
Comment on attachment 801346 [details]
pull request redirect to github

This is wrong. app transition depends on the orientation, all transition would fail to happen. https://bugzilla.mozilla.org/show_bug.cgi?id=908601
Attachment #801346 - Flags: feedback?(timdream)
Attachment #801346 - Flags: feedback?(alive)
Attachment #801346 - Flags: feedback-
(Assignee)

Comment 6

5 years ago
system related change will be done in bug 908601
(Assignee)

Comment 7

5 years ago
Comment on attachment 801346 [details]
pull request redirect to github

Since bug 908058 is landed, update PR for review.

Will left system related change to another bug
Attachment #801346 - Flags: review?(timdream)
DO NOT land this before 908601 since there would be a significant regression.
Comment on attachment 801346 [details]
pull request redirect to github

~/repo/gaia master$ grep -rl portrait-primary apps
apps/bluetooth/manifest.webapp
apps/calendar/manifest.webapp
apps/camera/manifest.webapp
apps/clock/manifest.webapp
apps/communications/manifest.webapp
apps/costcontrol/manifest.webapp
apps/email/manifest.webapp
apps/fm/manifest.webapp
apps/homescreen/manifest.webapp
apps/music/manifest.webapp
apps/pdfjs/manifest.webapp
apps/ringtones/manifest.webapp
apps/settings/manifest.webapp
apps/sms/manifest.webapp
apps/system/js/attention_screen.js
apps/system/js/cards_view.js
apps/system/js/lockscreen.js
apps/system/js/sleep_menu.js
apps/system/js/window.js
apps/system/js/window_manager.js
apps/system/style/system/system.css
apps/system/test/unit/cards_view_test.js
apps/wallpaper/manifest.webapp
apps/wappush/manifest.webapp
timdream:~/repo/gaia master$

Please make sure js file and test files are changed too.
Attachment #801346 - Flags: review?(timdream)

Updated

5 years ago
Whiteboard: [Flatfish only]
(Assignee)

Updated

5 years ago
Depends on: 908601
(Assignee)

Comment 10

5 years ago
Comment on attachment 801346 [details]
pull request redirect to github

add setringtone orientation, ask review again
Attachment #801346 - Flags: review?(timdream)
I am a bit confused. So the patch here does not cover orientation strings in JS and tests?
Flags: needinfo?(gasolin)
(Assignee)

Comment 12

5 years ago
after grep, only system has 'portrait-primary' orientation strings in JS and tests. I think bug 908601 covered that
Flags: needinfo?(gasolin)
Attachment #801346 - Flags: review?(timdream) → review+
(Assignee)

Comment 13

5 years ago
merged to gaia-master 18c2ab4b7b2d4cacee90fb85e97354142b2fad4a

thanks!
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-b2g-v1.2: --- → affected
Resolution: --- → FIXED
I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 18c2ab4b7b2d4cacee90fb85e97354142b2fad4a
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(gasolin)
(Assignee)

Comment 15

5 years ago
John, this issue depends on bug 908601
Flags: needinfo?(gasolin)
it seems like bug 908601 is already uplifted. can this bug now be uplifted? thanks
Flags: needinfo?(jhford)
There's still an issue uplifting:

$ git cherry-pick -x -m1 18c2ab4b7b2d4cacee90fb85e97354142b2fad4a
error: could not apply 18c2ab4... Merge pull request #12016 from gasolin/issue-911668
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
$ git diff
* Unmerged path apps/setringtone/manifest.webapp
Flags: needinfo?(jhford) → needinfo?(gasolin)
(Assignee)

Comment 18

5 years ago
uplift to 1.2 dbf61d49df1f4fdd3f9decf25575794b09e77d83
status-b2g-v1.2: affected → fixed
Flags: needinfo?(gasolin)
https://tbpl-dev.allizom.org/?tree=Mozilla-B2g26-v1.2&rev=54de309e18a9

The v1.2 patch have just turn the b2g26-v1.2 tree red. I just hit re-run if the same error show up we would have to back this patch out.
(Assignee)

Comment 21

5 years ago
thanks for check this
This was backed out of 1.2 for causing bug 934556:

https://github.com/mozilla-b2g/gaia/commit/3259ac8a9813740ffc43a5e4fdff8e4a024a56c8

Removing blocking-b2g flag so that this doesn't get uplifted to 1.2.
blocking-b2g: koi+ → ---
status-b2g-v1.2: fixed → wontfix
You need to log in before you can comment on or make changes to this bug.