559372 - Fennec start page still shows weave tabs option after logging out of Weave

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Tony Chung [:tchung]]

When you sync weave with fennec, the start page updates to include displaying tabs from other computers.  however, if you sign out of weave, the start page does not update correctly to remove that entry.  

The weave sync icon does gray out as i would expect it to.

See screenshot

Repro:
1) install fennec 1.1pre build 20100414
2) install weave 1.2
3) connect to a weave account and sync tabs
4) visit start page and notice fennec start page has an entry for "tabs from my other computers"
5) now log out of weave
6) restart
7) Verify after reopening, the weave sync icon in the left sidebar is grayed out, but the fennec start page still displays the entry of tabs from my other computers"

Expected:
- Fennec start page should not show any instance of tabs for weave if signed out. 

Actual:
- fennec start page still shows the remainder of the Tabs from my other computers entry

Comment 1

[Tony Chung [:tchung]]

Attachment: weave disable start page

Comment 2

[Tony Chung [:tchung]]

similar to bug 558923.  resetting blocking-fennec flag.

Comment 3

[Matt Brubeck (:mbrubeck)]

This is still true in Fennec 2.0a1pre with built-in Firefox Sync.

Also, "Desktop Bookmarks" still appears and contains data after signing out of Sync.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This does not resolve the bookmarks case but handle the start page case by enabling/disabled the row on the start page since weave is now built in (instead of hiding it)

The patch also handle the case where MOZ_SERVICES_SYNC is not defined

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)

> Also, "Desktop Bookmarks" still appears and contains data after signing out of
> Sync.

As it should. The bookmarks _are_ in Fennec now, so we don't need a live Sync connection to use them.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 468340 [details] [diff] [review]
Patch


>+    function updateWeaveButton() {
>+      let isDisabled = !getChromeWin().Weave.Service.isLoggedIn;
>+      document.getElementById("remoteTabs").setAttribute("disabled", isDisabled);

I think we should removeAttribute if isDisabled == false

I also wonder if we should just remove the remote tabs widget from the startup page, but that's a separate issue.

Comment 7

[Matt Brubeck (:mbrubeck)]

Do we really want this disabled on startup?  If it's enabled on startup, you can see your already-synced remote tabs right away, which seems useful even if they are not as up-to-date as possible.  We can disable the UI on logout, or if there is no username/password configured on startup.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #7)
> Do we really want this disabled on startup?  If it's enabled on startup, you
> can see your already-synced remote tabs right away, which seems useful even if
> they are not as up-to-date as possible.  We can disable the UI on logout, or if
> there is no username/password configured on startup.

I suppose we could be smarter if we know that Sync data exists locally.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #8)
> (In reply to comment #7)
> > Do we really want this disabled on startup?  If it's enabled on startup, you
> > can see your already-synced remote tabs right away, which seems useful even if
> > they are not as up-to-date as possible.  We can disable the UI on logout, or if
> > there is no username/password configured on startup.
> 
> I suppose we could be smarter if we know that Sync data exists locally.

The same is true for the new Awesomescreen. Instead of disabling the remote tabs button, we could do something smarter and look for local Sync data.

Followup bug?

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Do we really want this disabled on startup?  If it's enabled on startup, you
> > > can see your already-synced remote tabs right away, which seems useful even if
> > > they are not as up-to-date as possible.  We can disable the UI on logout, or if
> > > there is no username/password configured on startup.
> > 
> > I suppose we could be smarter if we know that Sync data exists locally.
> 
> The same is true for the new Awesomescreen. Instead of disabling the remote
> tabs button, we could do something smarter and look for local Sync data.
> 
> Followup bug?

http://hg.mozilla.org/mobile-browser/rev/1e4330671841


I've opened bug 590067 for that but I'm pretty sure we will need Madhava to decide what to show if there is no old data but the user go to the remote tabs panel: a quick explanation of how to configure weave and a link to redirect to the preference pane?

Comment 11

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b5pre) Gecko/20100825 Namoroka/4.0b5pre Fennec/2.0a1pre

and

Mozilla/5.0 (Android; Linux armv71; Nokia N900; en-US; rv:2.0b5pre) Gecko/20100825 Namoroka/4.0b5pre Fennec/2.0a1pre

Comment 12

[Tony Chung [:tchung]]

Assigning to self to create a litmus testcase.

Comment 13

[Reed Loden [:reed]]

(In reply to comment #12)
> Assigning to self to create a litmus testcase.

The assignee field is only for the developer who wrote the patch.

Comment 14

[Aakash Desai [:aakashd]]

We talked about this on irc with blassey and ted. We're experimenting.

Comment 15

[Tony Chung [:tchung]]

having the email address in in-litmus should work.

Comment 16

[Tony Chung [:tchung]]

Added litmus test: https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=12934