Selecting "Tabs from other computers" on home screen directs to Preferences

VERIFIED FIXED in Firefox 6

Status

defect
P2
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: breckard, Assigned: mbrubeck)

Tracking

({polish, regression, uiwanted})

Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox6 fixed, fennec6+)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Shortly after startup:

Selecting "Tabs from other computers" on home screen directs to Preferences menu and not the Sync menu.  If you wait a 5-count, this link does direct properly, but is confusing for the user in it's current form.

The link should at least direct to the Sync menu with a *working* or some other graphic. Not go to the Preferences menu.

This occurs on my Atrix for Beta5 and Fennec
This happens if you select this option during the first 5 seconds after Fennec starts, before Sync has connected.  Looks like a regression from bug 612607.
Assignee: nobody → mbrubeck
Blocks: 612607
Keywords: polish, regression
This also happens when the user is disconnected from the network and cannot connect to sync. Need a meaningful error message or a cache for the tabs from other computers.
Priority: -- → P2
Duplicate of this bug: 642575
Nominating for 4.next.
tracking-fennec: --- → ?
I've seen this as well on Fennec nightly, Droid2, when I clicked "Tabs" before sync finished connecting to the server.
tracking-fennec: ? → 2.0next+
Posted patch patch (obsolete) — Splinter Review
This fixes the problem for me, though I'm wondering if we should just remove this entire "else if" block.
Attachment #520275 - Flags: review?(wjohnston)
Whiteboard: [has patch]
My intent here was to catch the case where the user had set up sync, but had then disabled it via prefs. In that case, they don't need to set up sync again, just click the "enable" button.

With this patch I think we instead show old tabs that we had previously synced (unless logging in to sync has failed). Maybe that's better behavior anyway? I think maybe the ideal solution is to show the awesomescreen, but with a row giving some information about the current state of sync: "setting up", "disabled", etc? Adding madhava for feedback.
(In reply to comment #7)
> I think maybe the ideal solution is to show the awesomescreen, but with a row
> giving some information about the current state of sync: "setting up",
> "disabled", etc? Adding madhava for feedback.

I agree.  When sync is disabled or disconnected, I think the "remote tabs" awesomescreen panel should show the current status, with a button to change it ("Connect" or "Enable" depending on the status).
Status: NEW → ASSIGNED
Keywords: uiwanted
Yeah, agreed with c7 and c8. Flipping to the prefs panel is too ambiguous and frustrating for the user.

When a user switches to that awesomescreen tab (however they get there), we _could_ show the tabs that were current when the user was last connected, and reserve the first row for a "Sync is disabled" message and ability to turn it back on right there (thinking out loud here -- maybe we don't want to include the older taps?)  If they're simply not connected, we'll need to pop up the jpake dialog instead.
Re-read c8 a bit -- Matt, do you think we should have a message in the panel rather than popping up the jpake dialog if the user's not connected?
Just popping up the JPAKE dialog is probably fine too...  the downside is that it's more disruptive - you can't just switch back to a different awesomescreen panel.  (Maybe instead of hiding the awesomescreen as we do now, we should show the JPAKE dialog over the awesomescreen.)
Can the jpake dialog be shown inside a tab in the awesomescreen?
> (Maybe instead of hiding the awesomescreen as we do now, we should show
> the JPAKE dialog over the awesomescreen.)

This is Bug 640971
(In reply to comment #12)
> Can the jpake dialog be shown inside a tab in the awesomescreen?

I don't think we want to do that. The list is a list. Popping the dialog up over the list is fine.
Filed bug 644424 for the idea of showing sync status or connect / sync-now buttons in the remote tabs panel.
Depends on: 644424, 640971
Wes, do you think we could take this patch (or something similar) for Firefox 5, to improve the behavior in comment 0 and comment 2?  Then for Fx6 we can work on the approaches discussed in the later comments.
I'm not sure exactly what case this is catching. Looks like you are checking if we have the wrong username, and we actually do need to show some sort of error? The check here should look more like the ones here:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/sync.js#435

i.e. if sync is configured and enabled and set to autologin, then it doesn't matter we haven't logged in yet, we will soon... or something like that?
Posted patch patch v2 (obsolete) — Splinter Review
Updated based on Wes's comment.
Attachment #520275 - Attachment is obsolete: true
Attachment #520275 - Flags: review?(wjohnston)
Attachment #524628 - Flags: review?(wjohnston)
Attachment #524628 - Flags: review?(wjohnston) → review+
I found a problem with this patch; if we open the remote tabs list while tabs are still empty, then it never refreshes when sync later completes.
Whiteboard: [has patch] → [has patch][needs additional patch]
tracking-fennec: 2.0next+ → 6+
Posted patch patch v3 (obsolete) — Splinter Review
This will let the spinner keep spinning until login completes, then it will show the tabs.  There's one problem, though:  If it shows the JPAKE connection screen, and then the user presses cancel, the spinner will keep spinning forever.
Attachment #524628 - Attachment is obsolete: true
Posted patch patch v4Splinter Review
Show the spinner until sync setup/login completes or is aborted.
Attachment #534170 - Attachment is obsolete: true
Attachment #535225 - Flags: review?(wjohnston)
The remote tabs list is broken by an unrelated regression (bug 659777), so this patch cannot be properly tested without the fix from that bug.
Depends on: 659777
Whiteboard: [has patch][needs additional patch] → [has patch]
Comment on attachment 535225 [details] [diff] [review]
patch v4

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

Looks good and works great. Thanks Matt!

::: mobile/chrome/content/bindings.xml
@@ +1384,4 @@
>            this.setAttribute("loading", "true");
>  
> +          if (Weave.Service.isLoggedIn) {
> +            setTimeout(function() self._loadChildren());

Hmm. We don't need zero here anymore?
Attachment #535225 - Flags: review?(wjohnston) → review+
Pushed to cedar: http://hg.mozilla.org/projects/cedar/rev/3d845eea88e8

NOTE: This cannot be verified until this patch *and* bug 659777 are both merged to mozilla-central.


(In reply to comment #23)
> > +          if (Weave.Service.isLoggedIn) {
> > +            setTimeout(function() self._loadChildren());
> 
> Hmm. We don't need zero here anymore?

Oops, I left out the 0 by accident.  It actually seems to work fine that way, but I added it on checkin for clarity.
Whiteboard: [has patch] → [fixed-in-cedar]
Comment on attachment 535225 [details] [diff] [review]
patch v4

Requesting approval-mozilla-aurora to land this for Firefox 6 after it has been in Nightly for a while.  This is a highly visible polish issue (it affects anyone who tries to view desktop tabs immediately after Fennec starts) that has been triaged as blocking-fennec:2.0next+ and then tracking-fennec:6+.

The fix touches mobile files only, so there is no risk for desktop.
Attachment #535225 - Flags: approval-mozilla-aurora?
Pushed:
http://hg.mozilla.org/mozilla-central/rev/3d845eea88e8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [fixed-in-cedar]
Target Milestone: --- → Firefox 7
Attachment #535225 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed to Aurora for Firefox 6:
http://hg.mozilla.org/releases/mozilla-aurora/rev/90cd0558fc93
Target Milestone: Firefox 7 → Firefox 6
Awesome, thanks for taking my bug seriously! You guys rock!
Mozilla/5.0 (Android; Linux armv71; rv6.0a2) Gecko/20110623 Firefox/6.0a2 Fennec/6.0a2
Device: HTC Flyer
OS: Android 2.3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.