Closed
Bug 824161
Opened 11 years ago
Closed 11 years ago
Homescreen widget doesn't open awesomebar
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 21
People
(Reporter: wesj, Assigned: capella)
Details
Attachments
(1 file, 1 obsolete file)
1.38 KB,
patch
|
sriram
:
review+
|
Details | Diff | Splinter Review |
The homescreen widget only seems to actually open the awesomebar if Fennec was already running when I tapped it. It should load the Awesombar activity regardless.
Assignee | ||
Comment 1•11 years ago
|
||
Wow! new stuff I hadn't seen yet ... finally tracked it to https://bugzilla.mozilla.org/show_bug.cgi?id=708707. fwiw - this gives me my first reason to use widgets :D
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
This seems to work on my device ...
Attachment #696979 -
Flags: review?(wjohnston)
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 696979 [details] [diff] [review] Patch (v1) Review of attachment 696979 [details] [diff] [review]: ----------------------------------------------------------------- Missed this in my queue :( Looks pretty good, but I want to make sure we're not leaving blank tabs scattered around. ::: mobile/android/base/GeckoApp.java @@ +1947,5 @@ > + // Homescreen Widget requests awesome bar > + if (ACTION_WIDGET.equals(action)) { > + addTab(); > + } > + In this case I think we actually want to use the current tab... unless we're doing session restore or something. i.e. do a check like this: http://mxr.mozilla.org/mozilla-aurora/source/mobile/android/base/GeckoApp.java#1747 and alternate between addTab() and onSearchRequested()
Attachment #696979 -
Flags: review?(wjohnston) → review-
Assignee | ||
Comment 4•11 years ago
|
||
meh - small delay, and I figured you were busy with the release... I like the suggestion you made here. I've really not gotten into the onCreate() and initialize() functions too deeply yet, and had made a best-guess where to place the addTab(). This seems to work in my testing. I even ran eatmem crashes to test the restore situation. Let me know if you see a further refinement I should try.
Attachment #696979 -
Attachment is obsolete: true
Attachment #699551 -
Flags: review?(wjohnston)
Reporter | ||
Comment 5•11 years ago
|
||
Comment on attachment 699551 [details] [diff] [review] Patch (v2) Review of attachment 699551 [details] [diff] [review]: ----------------------------------------------------------------- Great! There's still a flash of the normal UI before the awesomescreen appears, but not showing that is probably going to require moving things around a lot in GeckoApp (i.e. we need to ensure that we know about the restore state and have initialized BrowserDB before we can even show UI). There's a risk for startup regressions there that I don't want to tackle here.
Attachment #699551 -
Flags: review?(wjohnston) → review+
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 699551 [details] [diff] [review] Patch (v2) Review of attachment 699551 [details] [diff] [review]: ----------------------------------------------------------------- I asked sriram to give this a look too, since he's the widget writer.
Attachment #699551 -
Flags: review+ → review?(sriram)
Comment 7•11 years ago
|
||
I have a weird feeling that the intent goes to onNewIntent() and that's how we load an external URL (say from twitter app). Could you please check that?
Assignee | ||
Comment 8•11 years ago
|
||
I can look again, but that's where I started my initial research. (I had a bunch of logcats in there to very the processing.) It's kinda not like a "new" intent if the activity hasn't been instantiated yet, so onCreate() grabs it. I'll look again >)
Reporter | ||
Comment 9•11 years ago
|
||
I'm pretty sure you're right about that too capella. ram was a bit worried about calling this before we call Tabs.getInstance().attachToActivity(this). Can you just move it towards the end of onCreate and put up a new patch for him to review? Technically, I think we're fine because we don't actually create a new tab until the Awesomebar returns (in case you just hit back we don't make and then kill a tab), but its good to be safe.
Assignee | ||
Comment 10•11 years ago
|
||
Help me here ... moving the code as suggested fails with a null pointer ... Moving the code from the end of |private initialize()| in geckoapp to the end of onCreate() doesn't move it downstream processing wise, but previously in the scheme of things. After onCreate() finishes is where onWindowFocusChanged() is called and in turn calls the initialize(). Maybe I mis-understand completely? The initialize() function in onCreate() referred to before the Tabs.getInstance().attachToActivity(this) seems to be to GeckoApplication. http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#1511
Reporter | ||
Comment 11•11 years ago
|
||
Oh you're right. I thought that's how things worked and was surprised to see the one early in onCreate(). If this is firing after all of onCreate() in geckoApp has run, I think we're fine then.
Assignee | ||
Comment 12•11 years ago
|
||
cooool :D I'll plan on moving the patch as already review+'ed ... thanks !!
Assignee | ||
Comment 13•11 years ago
|
||
Ah sorry, forgot I'm waiting now on sriram.
Comment 14•11 years ago
|
||
Comment on attachment 699551 [details] [diff] [review] Patch (v2) Review of attachment 699551 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #699551 -
Flags: review?(sriram) → review+
Assignee | ||
Comment 15•11 years ago
|
||
Push to try: https://tbpl.mozilla.org/?tree=Try&rev=f2de4fd29967
Assignee | ||
Comment 16•11 years ago
|
||
On to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/ca753cb40bb5
Comment 17•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca753cb40bb5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Updated•3 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
•