Closed
Bug 639334
Opened 13 years ago
Closed 13 years ago
Back button doesn't go back from pages opened by Fennec chrome
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: mbrubeck, Assigned: mbrubeck)
Details
(Keywords: polish, ux-mode-error, Whiteboard: [has patch][has review][needs approval])
Attachments
(2 files, 1 obsolete file)
3.73 KB,
patch
|
mbrubeck
:
review+
mfinkle
:
approval2.0+
|
Details | Diff | Splinter Review |
1.05 KB,
patch
|
Details | Diff | Splinter Review |
Android users have been complaining that sometimes the "back" button exits Fennec when they don't expect it to. One case where this happens is when a link is opened in a new tab by the Fennec chrome, including: * "Browse site" and "Open page" buttons in the add-on manager. * "Learn more" button in "What is an add-on?" row. * Feedback buttons in the Beta Tester Tools add-on. This patch makes the Android back button return to the previous page after these actions, instead of hiding Fennec and returning to the launcher screen. This is a very simple low-risk polish fix that addresses user feedback, so I think we should take it for Fennec 4.0.
Attachment #517276 -
Flags: review?(wjohnston)
Comment 1•13 years ago
|
||
Comment on attachment 517276 [details] [diff] [review] patch diff --git a/chrome/content/ContextCommands.js b/chrome/content/ContextCommands.js --- a/chrome/content/ContextCommands.js +++ b/chrome/content/ContextCommands.js @@ -61,7 +61,7 @@ var ContextCommands = { ContentAreaUtils.internalSave(popupState.mediaURL, null, null, popupState.contentDisposition, popupState.contentType, false, "SaveImageTitle", - null, browser.documentURI, false, null); + null, browser.documentURI, true, null); }, shareLink: function cc_shareLink() { Wrong patch. There are a few more of these that I saw: JPAKE Dialog: http://mxr.mozilla.org/mobile-browser/source/chrome/content/sync.js#539 GeoLocation Prompt: http://mxr.mozilla.org/mobile-browser/source/chrome/content/notification.xml#96
Attachment #517276 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 2•13 years ago
|
||
(In reply to comment #1) > - null, browser.documentURI, false, null); > + null, browser.documentURI, true, null); > Wrong patch. Fixed. > There are a few more of these that I saw: Added these, thanks.
Attachment #517307 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Attachment #517276 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Whiteboard: [has patch][needs review] → [has patch][has review][needs approval]
Assignee | ||
Comment 3•13 years ago
|
||
Related user feedback: http://input.mozilla.com/en-US/beta/opinion/1620092 http://input.mozilla.com/en-US/beta/opinion/1568247 http://input.mozilla.com/en-US/beta/opinion/1535700 http://input.mozilla.com/en-US/beta/opinion/1237292 http://input.mozilla.com/en-US/beta/opinion/1164110
Comment 4•13 years ago
|
||
Just ran into another one of these from prefs: http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.xul#420 It might be nice to use newOrSelectTab here as well.
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to comment #4) > Just ran into another one of these from prefs: Fixed this one locally, and one last one in downloads.js. Is there any case where we definitely do *not* want to set the opener. Perhaps we should just make this the default when aOpener is undefined?
Assignee | ||
Comment 6•13 years ago
|
||
This makes newTab(uri) default to using Browser.selectedTab as the opener. This is a simpler patch (touches less code), but represents a slight change in behavior for callers of this API.
Attachment #517463 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 517463 [details] [diff] [review] alternate patch The alternate patch may have some unwanted side effects. For Fennec 4.0 we should focus on the original patch, which is entirely localized to specific links in the UI.
Attachment #517463 -
Flags: review?(mark.finkle)
Comment 8•13 years ago
|
||
Comment on attachment 517307 [details] [diff] [review] patch v2 Can you file a new bug to make this more robust? More like the alt patch.
Attachment #517307 -
Flags: approval2.0+
Assignee | ||
Comment 9•13 years ago
|
||
http://hg.mozilla.org/mobile-browser/rev/dcbb11a046d6 Filed followup bug 639965.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 10•13 years ago
|
||
VERIFIED FIXED on: Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre Fennec /4.0b6pre Device: Motorola Droid 2 (Android 2.2)
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
tracking-fennec: ? → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•