If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Work - Show the Firefox Start page when the user taps/clicks the URL bar

RESOLVED FIXED

Status

Firefox for Metro
Browser
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

unspecified
All
Windows 8.1
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: feature=work)

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
Steps to reproduce:
1. Load a web page in Metro Firefox.
2. Display the toolbars, and tap/click in the location bar.
3. Type something.  The autocomplete popup should appear.
4. Delete what you typed.

Expected: The autocomplete popup disappears and you see the web page again.
Actual:   The autocomplete popup disappears and you see the Firefox start page.

This happens because StartUI.filter calls StartUI.show, but StartUI.unfilter fails to call StartUI.hide, so it leaves the start-container expanded.

We can avoid this problem in a familiar-looking way.  This is split out of the work I was doing on bug 836969.
(Assignee)

Comment 1

5 years ago
Created attachment 709375 [details] [diff] [review]
1/3: Rename bcast_changedViewState to bcast_windowState

This is just setting the stage.  I want to use a single broadcaster for several attributes that all affect the layout of the main UI, so I'm renaming it from bcast_changedViewState to bcast_windowState.
Attachment #709375 - Flags: review?(ally)
(Assignee)

Comment 2

5 years ago
Created attachment 709376 [details] [diff] [review]
2/2: Broadcast the "filtering" and "startpage" states

This separates the state of the startUI into two broadcasted attributes, one for the start page ("startpage") and one for the autocomplete popup ("filtering").

When either of these attributes is set, then we should show the start-container and hide the browser.  When filtering is set, then we show the autocomplete popup and hide the start page.

StartUI.show/hide now act only on the start page; they are no longer overloaded to affect the autocomplete popup.
Attachment #709376 - Flags: review?(ally)
(Assignee)

Comment 3

5 years ago
Created attachment 709382 [details] [diff] [review]
2/2: Broadcast the "filtering" and "startpage" states (v2)

I had removed the visible="true" from #tray in browser.xul, because on startup it would be out of sync with the ContextUI._visible variable, leaving the tray in an un-dismissable state.

However, this caused the tray to animate in when the start page was displayed on startup, which was janky and distracting.  It's better to keep the tray visible="true" on startup but make sure ContextUI has the right initial state.
Attachment #709376 - Attachment is obsolete: true
Attachment #709376 - Flags: review?(ally)
Attachment #709382 - Flags: review?(ally)
(Assignee)

Comment 4

5 years ago
Created attachment 709387 [details] [diff] [review]
2/2: Broadcast the "filtering" and "startpage" states (v3)

Fixed a minor layout bug, and simplified the patch a bit (you can ignore comment 3 now; I just dropped the changes related to that).  Sorry for the churn.
Attachment #709382 - Attachment is obsolete: true
Attachment #709382 - Flags: review?(ally)
Attachment #709387 - Flags: review?(ally)
rhoh, I can't reproduce the original behavior. Pulling new tree.
(Assignee)

Comment 6

5 years ago
Actually, based on item (4.2) from attachment 709174 [details] (bug 831899), maybe we *should* show the start page in this situation, and the bug is that we *don't* show it when you first tap in the URL bar.
(Assignee)

Comment 7

5 years ago
Changing the bug summary to
Blocks: 831899
No longer blocks: 831910
Summary: The start UI does not collapse when you delete the text in the URL bar → The Firefox Start page does not appear when the URL bar is first focused
(Assignee)

Comment 8

5 years ago
Created attachment 710227 [details] [diff] [review]
2/3: Broadcast the "filtering" and "startpage" states (v4)

...match the requirement from story bug 831899.  (Sorry, submitted too soon.)

Minor change to this patch to support this change.
Attachment #709387 - Attachment is obsolete: true
Attachment #709387 - Flags: review?(ally)
Attachment #710227 - Flags: review?(ally)
(Assignee)

Comment 9

5 years ago
Created attachment 710237 [details] [diff] [review]
3/3: Show the start page when the URL bar is focused

This brings us into compliance with the story as currently spec'ed:

* Show the start page automatically when the user taps the URL bar (BrowserUI._editURI).

* Hide it when the user presses escape while typing in the URL bar (BrowserUI.handleEscape) or navigates to a page (BrowserUI.showContent).

We want to make sure to hide the start page *immediately* when the user chooses a page to navigate to (rather than wait for the new page to load, which leads to a weird delay where the start page is still there for a second after the autocomplete popup goes away).  So we pass the URL that's *about* to load from goToURI to showContent to StartUI, so it can show/hide immediately based on the new page URL (rather than the current one).
Attachment #710237 - Flags: review?(ally)
(Assignee)

Updated

5 years ago
Attachment #709375 - Attachment description: 1/2: Rename bcast_changedViewState to bcast_windowState → 1/3: Rename bcast_changedViewState to bcast_windowState
(Assignee)

Updated

5 years ago
Summary: The Firefox Start page does not appear when the URL bar is first focused → Work - Show the Firefox Start page when the user taps/clicks the URL bar
Whiteboard: feature=work
(Assignee)

Comment 10

5 years ago
Created attachment 710242 [details] [diff] [review]
3/3: Show the start page when the URL bar is focused (v2)

Restore a line I accidentally changed, and add another comment.
Attachment #710237 - Attachment is obsolete: true
Attachment #710237 - Flags: review?(ally)
Attachment #710242 - Flags: review?(ally)
(Assignee)

Updated

5 years ago
Depends on: 838222
(Assignee)

Comment 11

5 years ago
Note: The latest patches are built on top of the one from bug 838222, so if you want to test at home you will need to import that patch first.
Comment on attachment 709375 [details] [diff] [review]
1/3: Rename bcast_changedViewState to bcast_windowState

looks good
Attachment #709375 - Flags: review?(ally) → review+
Comment on attachment 709387 [details] [diff] [review]
2/2: Broadcast the "filtering" and "startpage" states (v3)

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

- nit: you'll need to update the commit as this is 2/3 :)

- I'm uneasy about adding filtering & tabsonly to the broadcaster. One one hand, it does make for less code, on the other, I worry about leaky abstractions and overloading coming back to bite us. So, I'd like to hear your thinking on the topic a bit more (either vidyo, irc, or here is fine. Whichever is best for you)

- I'm also generally concerned about tests/testing for this, as we're 'sneaking' a bit of a refactor that could regress things into this bug. That said, I'm not sure a good automated one is possible or within scope right now.
Attachment #709387 - Flags: feedback+
Comment on attachment 710227 [details] [diff] [review]
2/3: Broadcast the "filtering" and "startpage" states (v4)

bah, commented on old patch. sorry about that
Attachment #710227 - Flags: review?(ally) → feedback+
Comment on attachment 710242 [details] [diff] [review]
3/3: Show the start page when the URL bar is focused (v2)

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

this part might be reasonable to write a test for :)

r+* - pending discussion on part 2
Attachment #710242 - Flags: review?(ally) → review+
(Assignee)

Updated

5 years ago
Blocks: 838364
Comment on attachment 710227 [details] [diff] [review]
2/3: Broadcast the "filtering" and "startpage" states (v4)

talked offline.
Attachment #710227 - Flags: feedback+ → review+
(Assignee)

Comment 17

5 years ago
http://hg.mozilla.org/projects/elm/rev/5a538865f0ef
http://hg.mozilla.org/projects/elm/rev/e6004e812395
http://hg.mozilla.org/projects/elm/rev/d27d4944ea92

I'll work on some tests tomorrow.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Flags: in-testsuite?
(Assignee)

Updated

5 years ago
Depends on: 841219

Updated

5 years ago
Blocks: 845152

Updated

5 years ago
No longer blocks: 831899
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.