Closed Bug 1023306 Opened 10 years ago Closed 10 years ago

Android homepage shortcuts don't work when the awesomescreen is open

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox30 affected, firefox31 affected, firefox32 affected)

RESOLVED FIXED
Firefox 33
Tracking Status
firefox30 --- affected
firefox31 --- affected
firefox32 --- affected

People

(Reporter: jaws, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

STR: Add a webpage shortcut to the Android homescreen Open a webpage with Firefox for Android Click on the URL to get to the awesomescreen Hit the Home button to minimze the app Click on the webpage shortcut that is on the Android homescreen ER: The webpage should be opened in a new tab AR: The awesomescreen is shown and the webpage is never loaded Probably a regression from whatever fixed bug 774280.
Blocks: 774280
Also 29 affected. I suspect this has always been the case.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0) > Probably a regression from whatever fixed bug 774280. Our awesomescreen code has totally changed since then, so I'm not sure whatever logic that fixed that bug is still around. We should probably dismiss edit mode when attempting to load the shortcut URL. Somehow this works properly when loading a URL from an external app (e.g. Twitter).
Comment on attachment 8439132 [details] [diff] [review] Always dismiss edit mode when opening external URLs (r=margaret) We were only dismissing edit mode: - For ACTION_VIEW intent action - After the page starts loading (the super.onNewIntent() call in BrowserApp) This means this bug also affects links opened from third-party apps. Furthermore, Homescreen bookmarks use a different intent action (GeckoApp.BOOKMARK). We have to dismiss edit mode before opening any external URL.
Attachment #8439132 - Flags: review?(margaret.leibovic)
(In reply to Lucas Rocha (:lucasr) from comment #4) > This means this bug also affects links opened from third-party apps. > Furthermore, Homescreen bookmarks use a different intent action > (GeckoApp.BOOKMARK). Not in the scope of this bug, but why do we even have this custom intent action? Couldn't we just use the ACTION_VIEW intent action for these shortcuts? Maybe a follow-up bug to investigate?
Comment on attachment 8439132 [details] [diff] [review] Always dismiss edit mode when opening external URLs (r=margaret) Review of attachment 8439132 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/BrowserApp.java @@ +2620,4 @@ > String action = intent.getAction(); > > + boolean externalUrl = Intent.ACTION_VIEW.equals(action) || > + GeckoApp.ACTION_BOOKMARK.equals(action); Make these local variable final? @@ +2625,5 @@ > + if (mInitialized && externalUrl) { > + // Dismiss editing mode if the user is loading a URL from an external app. > + mBrowserToolbar.cancelEdit(); > + > + Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.INTENT); This is changing the meaning of this telemetry probe, but perhaps we're okay with that. We could file a follow-up to add a new separate probe for this ACTION_BOOKMARK intent. It could be interesting to know how often people use these shortcuts (although perhaps creating the shortcut is the action we care about more).
Attachment #8439132 - Flags: review?(margaret.leibovic) → review+
Attachment #8439132 - Attachment is obsolete: true
Comment on attachment 8439835 [details] [diff] [review] Always dismiss edit mode when opening external URLs (r=margaret) (In reply to :Margaret Leibovic from comment #6) > Comment on attachment 8439132 [details] [diff] [review] > Always dismiss edit mode when opening external URLs (r=margaret) > > Review of attachment 8439132 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/BrowserApp.java > @@ +2620,4 @@ > > String action = intent.getAction(); > > > > + boolean externalUrl = Intent.ACTION_VIEW.equals(action) || > > + GeckoApp.ACTION_BOOKMARK.equals(action); > > Make these local variable final? Done. > @@ +2625,5 @@ > > + if (mInitialized && externalUrl) { > > + // Dismiss editing mode if the user is loading a URL from an external app. > > + mBrowserToolbar.cancelEdit(); > > + > > + Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.INTENT); > > This is changing the meaning of this telemetry probe, but perhaps we're okay > with that. We could file a follow-up to add a new separate probe for this > ACTION_BOOKMARK intent. It could be interesting to know how often people use > these shortcuts (although perhaps creating the shortcut is the action we > care about more). Good point. This new patch uses a different TelemetryContract.Method when you open a homescreen bookmark. Requesting feedback from liuche on the new Telemetry method.
Attachment #8439835 - Flags: review?(margaret.leibovic)
Attachment #8439835 - Flags: feedback?(liuche)
Comment on attachment 8439835 [details] [diff] [review] Always dismiss edit mode when opening external URLs (r=margaret) Review of attachment 8439835 [details] [diff] [review]: ----------------------------------------------------------------- Let's hear back from liuche about the telemetry change, but this looks good to me. Thanks. ::: mobile/android/base/BrowserApp.java @@ +2620,4 @@ > String action = intent.getAction(); > > + final boolean isViewAction = Intent.ACTION_VIEW.equals(action); > + final boolean isBookmarkAction = GeckoApp.ACTION_BOOKMARK.equals(action); Can we file a follow-up mentor bug to rename this intent ACTION_HOMESCREEN? Or even ACTION_HOMESCREEN_SHORTCUT. I find the "bookmark" name confusing, since this doesn't actually have anything to do with our bookmark system.
Attachment #8439835 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8439835 [details] [diff] [review] Always dismiss edit mode when opening external URLs (r=margaret) Review of attachment 8439835 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me! ::: mobile/android/base/BrowserApp.java @@ +2620,4 @@ > String action = intent.getAction(); > > + final boolean isViewAction = Intent.ACTION_VIEW.equals(action); > + final boolean isBookmarkAction = GeckoApp.ACTION_BOOKMARK.equals(action); +1
Attachment #8439835 - Flags: feedback?(liuche) → feedback+
(In reply to :Margaret Leibovic from comment #9) > Comment on attachment 8439835 [details] [diff] [review] > Always dismiss edit mode when opening external URLs (r=margaret) > > Review of attachment 8439835 [details] [diff] [review]: > ----------------------------------------------------------------- > > Let's hear back from liuche about the telemetry change, but this looks good > to me. Thanks. > > ::: mobile/android/base/BrowserApp.java > @@ +2620,4 @@ > > String action = intent.getAction(); > > > > + final boolean isViewAction = Intent.ACTION_VIEW.equals(action); > > + final boolean isBookmarkAction = GeckoApp.ACTION_BOOKMARK.equals(action); > > Can we file a follow-up mentor bug to rename this intent ACTION_HOMESCREEN? > Or even ACTION_HOMESCREEN_SHORTCUT. I find the "bookmark" name confusing, > since this doesn't actually have anything to do with our bookmark system. Filed bug 1026010.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: