The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in Firefox 6

Status

Fennec Graveyard
General
P2
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Bret Reckard, Assigned: mbrubeck)

Tracking

({polish, regression, uiwanted})

Trunk
Firefox 6
ARM
Android
polish, regression, uiwanted
Dependency tree / graph
Bug Flags:
in-testsuite ?

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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
(Assignee)

Comment 1

6 years ago
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
(Assignee)

Updated

6 years ago
Duplicate of this bug: 642575
(Assignee)

Comment 4

6 years ago
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.

Updated

6 years ago
tracking-fennec: ? → 2.0next+
(Assignee)

Comment 6

6 years ago
Created attachment 520275 [details] [diff] [review]
patch

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)
(Assignee)

Updated

6 years ago
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.
(Assignee)

Comment 8

6 years ago
(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?
(Assignee)

Comment 11

6 years ago
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.
(Assignee)

Comment 15

6 years ago
Filed bug 644424 for the idea of showing sync status or connect / sync-now buttons in the remote tabs panel.
Depends on: 644424, 640971
(Assignee)

Comment 16

6 years ago
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?
(Assignee)

Comment 18

6 years ago
Created attachment 524628 [details] [diff] [review]
patch v2

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+
(Assignee)

Comment 19

6 years ago
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+
(Assignee)

Comment 20

6 years ago
Created attachment 534170 [details] [diff] [review]
patch v3

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
(Assignee)

Comment 21

6 years ago
Created attachment 535225 [details] [diff] [review]
patch v4

Show the spinner until sync setup/login completes or is aborted.
Attachment #534170 - Attachment is obsolete: true
Attachment #535225 - Flags: review?(wjohnston)
(Assignee)

Comment 22

6 years ago
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+
(Assignee)

Comment 24

6 years ago
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]
(Assignee)

Comment 25

6 years ago
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
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [fixed-in-cedar]
Target Milestone: --- → Firefox 7
Attachment #535225 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 27

6 years ago
Pushed to Aurora for Firefox 6:
http://hg.mozilla.org/releases/mozilla-aurora/rev/90cd0558fc93
status-firefox6: --- → fixed
Target Milestone: Firefox 7 → Firefox 6
(Reporter)

Comment 28

6 years ago
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.