Closed Bug 827370 Opened 7 years ago Closed 7 years ago
When a web page is opened via an intent, we should not load about:blank first
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).
I notice this happening to me on Nightly as well. Same STR.
I think this is not a regression I can trace this back to Firefox 12
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.
This was originally fixed in bug 725660 (Firefox 13) so it must have regressed since then.
Firefox 18 is not affected, but Firefox 19 (Beta) is 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
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+
Filed bug 837233 for adding the RedirectorRunnable back.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
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
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.
(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.
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.
You need to log in before you can comment on or make changes to this bug.