When a web page is opened via an intent, we should not load about:blank first

VERIFIED FIXED in Firefox 19

Status

()

VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: jaws, Assigned: bnicholson)

Tracking

(Blocks: 1 bug, {polish, regression, ux-minimalism})

Trunk
Firefox 21
ARM
Android
polish, regression, ux-minimalism
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18 unaffected, firefox19+ verified, firefox20+ verified, firefox21 verified)

Details

Attachments

(1 attachment)

STR:
Use an external app like Twitter to open a link with Firefox
When Firefox opens, notice that the address bar shows about:blank (easier to see on 2G)
Then the web page starts to load.

There are either two things that could be done here. First, we shouldn't show "about:blank" in the address bar. It is technical information that isn't valuable to the user, and gives the user the idea that Firefox is doing unnecessary work.

Second, maybe we don't need to load about:blank here (although I don't know what we would do in its absence).
Bug 729911 / Bug 725660? Sounds like a regression.
Keywords: regression

Updated

6 years ago
tracking-fennec: --- → ?
I notice this happening to me on Nightly as well. Same STR.
Keywords: regressionwindow-wanted

Comment 3

6 years ago
I think this is not a regression I can trace this back to Firefox 12
Keywords: regressionwindow-wanted
There are two things here; showing 'about:blank' in the address bar was once fixed and is now re-occuring, regression. Actually showing 'about:blank' in content, is not, as it's always been there. Let's use this bug for finding the regression for showing 'about:blank' the string, in the address-bar.
Keywords: regressionwindow-wanted
Blocks: 814110
This was originally fixed in bug 725660 (Firefox 13) so it must have regressed since then.
Blocks: 725660
Firefox 18 is not affected, but Firefox 19 (Beta) is affected.
status-firefox18: --- → unaffected
status-firefox19: --- → affected
status-firefox20: --- → affected
status-firefox21: --- → affected
I think this is a regression from bug 800199.  I confirmed using m-c nightlies that the regression is in this range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5
Blocks: 800199
Keywords: regressionwindow-wanted → polish, ux-minimalism
(Assignee)

Updated

6 years ago
Assignee: nobody → bnicholson
(Assignee)

Comment 8

6 years ago
Created attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

As is, the RedirectorRunnable code is buggy and not working as intended. The overridden afterLoad() method isn't even being called since the method is private: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#1964. This is responsible buggy behavior I'm seeing when launching Twitter shortcuts, such as:
1) about:blank showing up for a long time before the page loads
2) occasionally, *only* about:blank loads and page never loads
3) onNewIntent() is fired many times (often 20+)

The bug as filed happens because when using a RedirectorRunnable, we first create an empty (about:blank) tab, then send a Tab:Load event to Gecko on the empty tab once we know the redirected URL.

Even with the fixed overridden method, I'm not seeing any performance benefits when using the RedirectorRunnable. Since it complicates the startup code, I think we should just get rid of it. I'm not sure how much the PrefetchRunnable code is helping either, but I kept it since it doesn't interfere with startup like the RedirectorRunnable.

I did, however, remove the PrefetchRunnable code from onNewIntent(). We're creating a PrefetchRunnable in two places (the other being initialize), but I don't think there's much benefit in creating one if Gecko is already running.
Attachment #708812 - Flags: review?(mark.finkle)
Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

It's questionable whether Prefetch has any impact, but it's straight forward. Spinning up the modem before Gecko starts (and needs to spin up the modem) seems like a common thing and a potential win.

Only using it when we initially launch seems like a good idea, but we might find that we're in the background long enough between uses that the modem winds down between Firefox uses. Maybe for those cases we'd be OOM'd in the background though.

We might want to add telemetry (in a followup bug) for checking modem state in onNewIntent.

RedirectorRunnable is a good idea, but I think the complexity it adds to startup causes more than a few issues.
Attachment #708812 - Flags: review?(mark.finkle) → review+
(Assignee)

Updated

6 years ago
status-firefox21: affected → fixed
(Assignee)

Comment 11

6 years ago
Filed bug 837233 for adding the RedirectorRunnable back.
https://hg.mozilla.org/mozilla-central/rev/bb11e1f204ca
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
(Assignee)

Comment 13

6 years ago
Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 800199
User impact if declined: startup UI/performance problems (see comment 8)
Testing completed (on m-c, etc.): m-c a few days
Risk to taking this patch (and alternatives if risky): low risk - just removes broken code
String or UUID changes made by this patch: none
Attachment #708812 - Flags: approval-mozilla-beta?
Attachment #708812 - Flags: approval-mozilla-aurora?

Updated

6 years ago
tracking-firefox19: --- → +
tracking-firefox20: --- → +
Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

FF19 regression with a very low risk fix. Please land asap today.

I'm assuming that opening links from outside Firefox would be the best way to verify. Any other direction for QA would be helpful.
Attachment #708812 - Flags: approval-mozilla-beta?
Attachment #708812 - Flags: approval-mozilla-beta+
Attachment #708812 - Flags: approval-mozilla-aurora?
Attachment #708812 - Flags: approval-mozilla-aurora+

Updated

6 years ago
Keywords: qawanted, verifyme
(Assignee)

Comment 15

6 years ago
(In reply to Alex Keybl [:akeybl] from comment #14)
> Comment on attachment 708812 [details] [diff] [review]
> Remove RedirectorRunnable
> 
> FF19 regression with a very low risk fix. Please land asap today.
> 
> I'm assuming that opening links from outside Firefox would be the best way
> to verify. Any other direction for QA would be helpful.

The best way to test this is to load links from Twitter (t.co links) to open Fennec from a cold state.
On cold-start (Fennec not running), opening a URL via an intent, this is fixed. When Fennec is already running and I open a URL via an intent, I continue to see 'about:blank' flash in the title bar. Testing on 02/05 Nightly.
(In reply to Aaron Train [:aaronmt] from comment #18)
> On cold-start (Fennec not running), opening a URL via an intent, this is
> fixed. When Fennec is already running and I open a URL via an intent, I
> continue to see 'about:blank' flash in the title bar. Testing on 02/05
> Nightly.

Let's open a new bug for that case.

Comment 21

6 years ago
Verified using Asus Transformer TF101 (Android 4.0.3) on:
Firefox 19.0b2 build 2 (2013-02-05), Nightly 21.0a1(2013-02-07) and Aurora 20.0a2(2013-02-07).
The behaviour is the same as Aaron describe it in comment 18.
Status: RESOLVED → VERIFIED
status-firefox19: fixed → verified
status-firefox20: fixed → verified
status-firefox21: fixed → verified

Updated

6 years ago
Keywords: qawanted

Updated

6 years ago
Keywords: verifyme
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.