Closed Bug 835610 Opened 7 years ago Closed 7 years ago

Work - [polish] Firefox app bar location text does not revert to the previous value when user presses escape

Categories

(Firefox for Metro Graveyard :: Browser, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: mbrubeck, Assigned: ally)

References

Details

(Keywords: platform-parity, polish, ux-error-recovery, Whiteboard: feature=work)

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:
1. Load a page in Metro Firefox.
2. After the page loads, focus the location bar.
3. Type a word.
4. Press the Escape key.

Expected results: The URL bar reverts to the address of the current page.

Actual results: The URL bar hides, but still contains the typed word.
Summary: [polish] Metro url bar text does not revert to the previous value when user presses escape → Work - [polish] Metro url bar text does not revert to the previous value when user presses escape
Whiteboard: feature=work
Summary: Work - [polish] Metro url bar text does not revert to the previous value when user presses escape → Work - [polish] Firefox app bar location text does not revert to the previous value when user presses escape
Blocks: 845152
No longer blocks: 831899
Assignee: mbrubeck → nobody
Assignee: nobody → ally
- I left the new {} braces in one case of handleEscape() because it is in line with our style guide.
- I left some of the more interesting error console logging for you in case you'd like to see the output during feedback.
- I think lastKnownGoodURL should move into browserUI, and I welcome better names for that variable. :/
Attachment #744902 - Flags: feedback?(mbrubeck)
Comment on attachment 744902 [details] [diff] [review]
wip - functional, needs some code hygiene

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

Yeah, this looks good to me.

::: browser/metro/base/content/browser-ui.js
@@ +57,5 @@
>    });
>  });
>  
> +let lastKnownGoodURL = ""; //used when the user wants to escape unfinished url entry
> +// TODO I think this belongs in browserUI,not out here with Elements

I agree.
Attachment #744902 - Flags: feedback?(mbrubeck) → feedback+
Attachment #744902 - Attachment is obsolete: true
Attachment #746051 - Flags: review?(mbrubeck)
Comment on attachment 746051 [details] [diff] [review]
save urls when they are set

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

::: browser/metro/base/content/browser-ui.js
@@ +77,4 @@
>    get _back() { return document.getElementById("cmd_back"); },
>    get _forward() { return document.getElementById("cmd_forward"); },
>  
> +  lastKnownGoodURL : "", //used when the user wants to escape unfinished url entry

Nit: no space before ":"

No rationale except consistency.  Mozilla JS code overall is divided on this issue, but this is the dominant style in Metro so I've added it to https://wiki.mozilla.org/Firefox/Windows_8_Metro_Style_Guides#JS
Attachment #746051 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/mozilla-central/rev/aa00d8de9a79
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.