Closed Bug 891056 Opened 6 years ago Closed 6 years ago

Defect - [8.1] Support "snapped view" in Windows 8.1

Categories

(Firefox for Metro Graveyard :: App Bar, defect, P2)

All
Windows 8.1
defect

Tracking

(firefox26 fixed, firefox27 fixed)

VERIFIED FIXED
Firefox 27
Tracking Status
firefox26 --- fixed
firefox27 --- fixed

People

(Reporter: Samvedana, Assigned: mbrubeck)

References

Details

(Whiteboard: [8.1] feature=defect c=firefox_start u=metro_firefox_user p=3)

Attachments

(8 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:25.0) Gecko/20130707 Firefox/25.0
Build ID: 20130707031138

Tested  for iteration-9 on Windows 8.1 preview using latest nightly build from ftp://ftp.mozilla.org/pub/firefox/nightly/2013/07/2013-07-07-03-11-38-mozilla-central/

STR and expected result:
1.A Metro Firefox user launches Metro Firefox to the Awesome screen and swipes in the app bars.
2.Using the Windows standard gesture, the user puts Firefox into snapped view.
The user sees the app bars disappear, the Top Sites tile group's thumbnail tiles turn into standard tiles, and Bookmarks tile group and History tile group are moved from their location to the right of the Top Sites tile group to the location underneath it.
3.Using the Windows standard gesture, the user puts Firefox into fill view and full screen view.
4.The user sees the app bars return and the Awesome screen re-configured with the Bookmarks tile group and the History tile group to the right of the Top Sites tile group.

Actual result:
When I am doing step-2, "Using the Windows standard gesture, the user puts Firefox into snapped view", I see the app bars don't disappear.
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0
For step-3, Windows 8.1 doesn't support fill view. We can adjust screen according to our needs.

"Windows 8.1 Preview does not have fixed-width view states. Users can now resize apps continuously down to a minimum width. (The default minimum width of an app is 500 pixels.) So apps no longer have the snapped and fill view states. Instead, you develop your app to be functional and good looking at any size down to the minimum" from http://msdn.microsoft.com/en-us/library/windows/apps/bg182890.aspx
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0
Summary: Defect- Use Firefox Start in snapped view → Defect - Use Firefox Start in snapped view
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0
(In reply to Samvedana (:Samvedana) from comment #1)
> For step-3, Windows 8.1 doesn't support fill view. We can adjust screen
> according to our needs.
> 
> "Windows 8.1 Preview does not have fixed-width view states. Users can now
> resize apps continuously down to a minimum width. (The default minimum width
> of an app is 500 pixels.) So apps no longer have the snapped and fill view
> states. Instead, you develop your app to be functional and good looking at
> any size down to the minimum" from
> http://msdn.microsoft.com/en-us/library/windows/apps/bg182890.aspx

Samvedana, would you mind filing a separate 8.1 specific bug on this, so we can concentrate on the 8.0 bug here with the app bars?
Let's use this bug for the 8.1 support.  This ties into a change I was already planning to make.
Assignee: nobody → mbrubeck
Summary: Defect - Use Firefox Start in snapped view → Defect - Support "snapped view" in Windows 8.1
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0 → feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1]
Flags: needinfo?(samvedana.gohil)
(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> Let's use this bug for the 8.1 support.  This ties into a change I was
> already planning to make.

Do we still need a new bug on the 8.0 issue sgohil mentions in comment 0?

"When I am doing step-2, "Using the Windows standard gesture, the user puts Firefox into snapped view", I see the app bars don't disappear."
(In reply to Jim Mathies [:jimm] from comment #4)
> Do we still need a new bug on the 8.0 issue sgohil mentions in comment 0?

Comment 0 was about testing done on Windows 8.1 preview.  I don't see any Windows 8.0 issues mentioned in this bug.
Summary: Defect - Support "snapped view" in Windows 8.1 → Defect - [8.1] Support "snapped view" in Windows 8.1
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1] → [8.1] feature=defect c=firefox_start u=metro_firefox_user p=0
Depends on: 892512
Priority: -- → P2
Flags: needinfo?(samvedana.gohil)
Whiteboard: [8.1] feature=defect c=firefox_start u=metro_firefox_user p=0 → feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1]
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1] → feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1][preview-triage]
Whiteboard: feature=defect c=firefox_start u=metro_firefox_user p=0 [8.1][preview-triage] → [8.1] [preview-triage] feature=defect c=firefox_start u=metro_firefox_user p=0
Blocks: metrov1backlog
No longer blocks: metrov1defect&change
Whiteboard: [8.1] [preview-triage] feature=defect c=firefox_start u=metro_firefox_user p=0 → [8.1] [preview-triage][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=0
Summary: Defect - [8.1] Support "snapped view" in Windows 8.1 → [MP] Defect - [8.1] Support "snapped view" in Windows 8.1
Whiteboard: [8.1] [preview-triage][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=0 → [8.1] [preview][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=0
p=3
Status: NEW → ASSIGNED
Whiteboard: [8.1] [preview][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=0 → [8.1] [preview][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=3
Version: 25 Branch → Trunk
Blocks: metrov1it14
No longer blocks: metrov1backlog
QA Contact: jbecerra
Whiteboard: [8.1] [preview][blocked until 8.1 is out] feature=defect c=firefox_start u=metro_firefox_user p=3 → [8.1] [preview] feature=defect c=firefox_start u=metro_firefox_user p=3
Depends on: 898650
Attached patch part 1: clean upSplinter Review
Removes some unused CSS for #start-autocomplete (which doesn't exist anymore) and replace some JavaScript code with CSS.
Attachment #796851 - Flags: review?(rsilveira)
Comment on attachment 796851 [details] [diff] [review]
part 1: clean up

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

Looks great and also fixes Bug 907935!
Attachment #796851 - Flags: review?(rsilveira) → review+
Attachment #796984 - Flags: review?(rsilveira)
Comment on attachment 796984 [details] [diff] [review]
part 2: use resize events instead of Windows system messages

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

The start grid views are still depending on metro_viewstate_changed. With this change they can get out of sync with the windowState broadcaster.

I think we could still keep a viewstate changed event/broadcaster that gets sent by contentAreaObserver when it changes. We can then use it to update the views, hide the flyout panels and on StartUI instead of resize. Thoughts?

::: browser/metro/base/content/ContentAreaObserver.js
@@ +77,5 @@
>      return this._deckTransitioning;
>    },
>  
> +  get viewstate() {
> +    if (window.innerWidth < Services.prefs.getIntPref("browser.ui.snapped.maxWidth")) {

Nit: You can use the width/height properties defined in ContentAreaObserver.

@@ +292,5 @@
> +  _updateViewState: function() {
> +    let viewstate = this.viewstate;
> +    Elements.windowState.setAttribute("viewstate", viewstate);
> +    if (viewstate == "snapped") {
> +      FlyoutPanelsUI.hide();

This feels out of place here. We can move this logic to FlyoutPanelsUI if we have a viewstate changed event.

::: browser/metro/base/content/startui/StartUI.js
@@ +110,4 @@
>      }
>    },
>  
> +  get viewstate() {

Use this.chromeWin.ContentAreaObserver.viewstate instead.
Attachment #796984 - Flags: review?(rsilveira) → review-
Whiteboard: [8.1] [preview] feature=defect c=firefox_start u=metro_firefox_user p=3 → [8.1] [preview] feature=defect c=firefox_start u=metro_firefox_user p=3 [leave open]
Blocks: metrov1it15
No longer blocks: metrov1it14
Addresses review comments.  Moves some shared view code into the "View" superclass.
Attachment #796984 - Attachment is obsolete: true
Attachment #808961 - Flags: review?(rsilveira)
Note: I'm still finishing up part 4, which updates our tests.
Attachment #808971 - Flags: review?(netzen)
Comment on attachment 808971 [details] [diff] [review]
part 5: remove unused widget code

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

r+ assuming you're changing this code in other patches:
http://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=snappedState&redirect=true

I noticed you had other patches but didn't check them myself.

::: widget/nsIWinMetroUtils.idl
@@ -51,5 @@
>  
> -  /**
> -   * Attempts to unsnap the application from snapped state to filled state
> -   */
> -   void unsnap();

I think it's the right thing to do to remove the ability for front end to try to do this since it has no effect on 8.1, even know it may be useful in 8.0.  Likewise I'm glad you left the MetroUtils code in place which is used by the filepicker so that it still works as it should on 8.0.
Attachment #808971 - Flags: review?(netzen) → review+
Comment on attachment 808961 [details] [diff] [review]
part 2: use resize events instead of Windows system messages (v2)

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

/me claps
Looks great and works great. It will make testing snapped/portrait testing in desktop mode much easier too \o/. Thanks for taking this.
Attachment #808961 - Flags: review?(rsilveira) → review+
Attachment #808962 - Flags: review?(rsilveira) → review+
Attachment #809243 - Flags: review?(rsilveira) → review+
Comment on attachment 808961 [details] [diff] [review]
part 2: use resize events instead of Windows system messages (v2)

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

Yeah looks good. Many +1s for easier testing and troubleshooting in -metrodesktop mode. Also, I figured it was a matter of time before View sprouted a constructor :) Eventually I'd like to get the various fetch items and populate grid view methods lined up so only the details of the different data providers need be in the individual views.
Attachment #808961 - Flags: feedback+
Blocks: metrov1it16
No longer blocks: MetroPreviewRelease, metrov1it15
Summary: [MP] Defect - [8.1] Support "snapped view" in Windows 8.1 → Defect - [8.1] Support "snapped view" in Windows 8.1
Whiteboard: [8.1] [preview] feature=defect c=firefox_start u=metro_firefox_user p=3 [leave open] → [8.1] feature=defect c=firefox_start u=metro_firefox_user p=3 [leave open]
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Hardware: x86_64 → All
Resolution: --- → FIXED
Whiteboard: [8.1] feature=defect c=firefox_start u=metro_firefox_user p=3 [leave open] → [8.1] feature=defect c=firefox_start u=metro_firefox_user p=3
Target Milestone: --- → Firefox 27
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Release of Windows 8.1

User impact if declined: Firefox UI renders incorrectly in split-screen mode in Windows 8.1, including significant usability problems with navigation toolbar and start screen.

Testing completed (on m-c, etc.): Landed on m-c on 9/24.

Risk to taking this patch (and alternatives if risky): This is a large patch for Metro; there is some risk of Metro regressions that we haven't caught yet.  We could wait for more testing on m-c if we want to minimize this.  The patch does not touch any non-Metro code, so there is no risk to desktop Firefox or other products.

String or IDL/UUID changes made by this patch:  None.  (I left out the "part 5" patch because it is not required on Aurora and it changed an IDL file.)
Attachment #811271 - Flags: review+
Attachment #811271 - Flags: approval-mozilla-aurora?
Comment on attachment 811271 [details] [diff] [review]
rollup of parts 2-4 for Aurora approval

Touches only Metro code and has been on central over a week - approving for Aurora so it can bake there too.
Attachment #811271 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attached image screenshot_Nightly.png
While testing this for iteration #16, with latest Nightly (build ID: 20131021030203), on both Win 8.1 32-bit and 64-bit, Metro in snapped view looks like in the attached screenshot, with the URL bar (navigation app bar) visible.

Isn't this expected?
Flags: needinfo?(mbrubeck)
Attached image screenshot_Aurora.png
I've encountered another issue:  with latest Aurora (build ID: 20131021004002), on both Win 8.1 32-bit and 64-bit, after making it the default browser, when entering in Metro mode, I'm stucked at the screen shown in this attachement.

Does anyone have any thoughts/suggestions? Thanks!
(In reply to Manuela Muntean [:Manuela] [QA] from comment #25)
> While testing this for iteration #16, with latest Nightly (build ID:
> 20131021030203), on both Win 8.1 32-bit and 64-bit, Metro in snapped view
> looks like in the attached screenshot, with the URL bar (navigation app bar)
> visible.

Note that Windows 8.1 does not have "snapped" view like Windows 8 did.  We now use our special "snapped view" layout only when the window width is less than 600px.  ("Narrow view" might be a better name.)  Otherwise we use either our portrait or landscape layout, depending on the window width and height.

(In reply to Manuela Muntean [:Manuela] [QA] from comment #26)
> I've encountered another issue:  with latest Aurora (build ID:
> 20131021004002), on both Win 8.1 32-bit and 64-bit, after making it the
> default browser, when entering in Metro mode, I'm stucked at the screen
> shown in this attachement.

This usually means the browser is crashing on startup, or otherwise completely failing to start -- probably an unrelated bug in Aurora.
Flags: needinfo?(mbrubeck)
> Note that Windows 8.1 does not have "snapped" view like Windows 8 did.  We
> now use our special "snapped view" layout only when the window width is less
> than 600px.  ("Narrow view" might be a better name.)  Otherwise we use
> either our portrait or landscape layout, depending on the window width and
> height.

Marking this verified based on Matt's comment from above and comment 25.
Status: RESOLVED → VERIFIED
Went through the following "Defect" for iteration #19 without some issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-11-27-03-02-01-mozilla-central/

- Ensured that "Full View" works and everything is being rendered correctly
- Ensured that the "50/50" view works and everything is being rendered correctly
- Ensured that the user can change the size of the window either left or right without any issues
- Ensured that there's no rendering issues when resizing the Firefox Metro view
- Ensured that the content in Firefox Metro is being displayed/rendered correctly when resized
- Ensured that you can use both Windows Desktop/Firefox Metro when using the side-by-side view (50/50)

Issues:

- When going through the different views available, some of the pages will be cut off under the awesome screen. Sometimes it also renders the page differently. Created Bug 944255
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.