Closed Bug 1048545 Opened 10 years ago Closed 10 years ago

Tapping the branding logo in the browser widget opens a new tab

Categories

(Firefox for Android Graveyard :: General, defect, P1)

34 Branch
ARM
Android
defect

Tracking

(firefox34 affected, firefox35 verified, fennec+)

VERIFIED FIXED
Firefox 34
Tracking Status
firefox34 --- affected
firefox35 --- verified
fennec + ---

People

(Reporter: aaronmt, Assigned: wesj)

References

Details

Attachments

(1 file)

(In reply to Anthony Lam (:antlam) from bug 1035642 comment #23)
> "Is it expected that tapping the branding logo also opens a new tab?"

> In my head, I would think tapping the logo would open up Firefox (just like
> tapping on the app icon would do in the app drawer)

Currently, tapping the browser icon in the widget opens a new tab.
I'm a little confused by the title of this bug. I think it was my fault so let me clear up what I meant earlier :)

I think tapping the build icon in the widget should act the same as tapping the app icon in the app drawer. This could be a "new tab" or it could be where ever the user left off last (providing they did not kill the app in the "recents" tray).
> I think tapping the build icon in the widget should act the same as tapping the app icon in the app drawer.

That just opens the browser, it does not open a new tab.
http://dxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/SearchWidget.java#99

Line #99 defines the same action as new tab .. if that's what you want .. I'm just double checking in this bug that's all
(In reply to Aaron Train [:aaronmt] from comment #2)
> > I think tapping the build icon in the widget should act the same as tapping the app icon in the app drawer.
> 
> That just opens the browser, it does not open a new tab.

I think it should just open the browser since "+" already opens a new tab.
I think I forgot to ping wesj on this bug so NI-ing him here :)
Flags: needinfo?(wjohnston)
wesj fix me please!

Or if you don't maybe I'll write a patch :)
Assignee: nobody → wjohnston
tracking-fennec: --- → +
Blocks: search
Priority: -- → P1
Attached patch PatchSplinter Review
I did this right for once (I think?) Filed a pull request too:
https://github.com/mozilla/fennec-search/pull/62
Attachment #8475349 - Flags: review?(margaret.leibovic)
Flags: needinfo?(wjohnston)
Comment on attachment 8475349 [details] [diff] [review]
Patch

Review of attachment 8475349 [details] [diff] [review]:
-----------------------------------------------------------------

r- because I think we can make this better to avoid messing up our telemetry data.

::: mobile/android/search/java/org/mozilla/search/SearchWidget.java
@@ +67,5 @@
>                      AppConstants.ANDROID_PACKAGE_NAME,
>                      AppConstants.BROWSER_INTENT_CLASS_NAME,
>                      intent);
> +        } else if (intent.getAction().equals(ACTION_LAUNCH_NEW_TAB)) {
> +                redirect = buildRedirectIntent(Intent.ACTION_VIEW,

Interesting... so the new tab logic is basically just telling Fennec to open an empty URL. Will this ever break? Should we create a more explicit "new tab" intent?

Looking at our ACTION_VIEW handling code, we're actually recording a telemetry "load url" event for this case, which is probably not optimal. If we had a separate "new tab" intent, we could add a probe to see how often users tap that. In fact, we should add probes directly to this search widget to see how much users are using it.
Attachment #8475349 - Flags: review?(margaret.leibovic) → review-
(In reply to :Margaret Leibovic from comment #9)
> Interesting... so the new tab logic is basically just telling Fennec to open
> an empty URL. Will this ever break? Should we create a more explicit "new
> tab" intent?

I think it asks to load "about:home". Not an empty url. Any ACTION_VIEW intent that comes in opens a new tab. Thats not my favorite behavior (I'd rather switch-to-tab if we can), but its by design.

I don't think we need a new tab intent.
(In reply to Wesley Johnston (:wesj) from comment #10)
> (In reply to :Margaret Leibovic from comment #9)
> > Interesting... so the new tab logic is basically just telling Fennec to open
> > an empty URL. Will this ever break? Should we create a more explicit "new
> > tab" intent?
> 
> I think it asks to load "about:home". Not an empty url. Any ACTION_VIEW
> intent that comes in opens a new tab. Thats not my favorite behavior (I'd
> rather switch-to-tab if we can), but its by design.

I'm sorry, I was wrong, I didn't look at this code closely enough. You're right.

> I don't think we need a new tab intent.

I still don't know how I feel about us recording a telemetry event for this, though, since right now we're recording this as a "user loaded a url from an intent" event. Right now we treat those data points as "users opened external links in fennec", but that's not really what this is. I suppose if we add telemetry probes in this SearchWidget logic we can subtract those numbers from the overall set of "url opened from intent" events to get a sense of how many external links were opened.
Comment on attachment 8475349 [details] [diff] [review]
Patch

Thinking about this more, it's fine to land this, since the point of this bug is to fix the interaction clicking on the browser logo.

However, I'd like to see a follow-up bug to address the telemetry issue.
Attachment #8475349 - Flags: review- → review+
https://hg.mozilla.org/mozilla-central/rev/341e503cabf9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Verified as fixed in build 35.0a1 (2014-09-08);
Devices: 
Google Nexus 7 (Android 4.4.4);
Samsung Galaxy R (Android 2.3.4).
Status: RESOLVED → VERIFIED
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: