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)
Firefox for Android Graveyard
General
Tracking
(firefox30 affected, firefox31 affected, firefox32 affected)
RESOLVED
FIXED
Firefox 33
People
(Reporter: jaws, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
3.33 KB,
patch
|
Margaret
:
review+
liuche
:
feedback+
|
Details | Diff | Splinter Review |
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.
Comment 1•10 years ago
|
||
Also 29 affected. I suspect this has always been the case.
Comment 2•10 years ago
|
||
(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 3•10 years ago
|
||
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
(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 6•10 years ago
|
||
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+
Comment 7•10 years ago
|
||
Updated•10 years ago
|
Attachment #8439132 -
Attachment is obsolete: true
Comment 8•10 years ago
|
||
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 9•10 years ago
|
||
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 10•10 years ago
|
||
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+
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
(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.
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•