868523 - snapped synced tabs entry doesnt hide if sync is not enabled

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Firefox Start, defect)

Description

[Allison Naaktgeboren :ally]

It occurs to me that we hide the synced tabs header when sync is not enabled on the full size Awesome screen, but not for the snapped one. We should be consistent.

Comment 1

[Matt Brubeck (:mbrubeck)]

I think Jim may have noticed and fixed this independently in bug 868545.

Comment 2

[Allison Naaktgeboren :ally]

Jimm's manipulating the sync logic & when it occurs, this is that I wrote a (now) limiting assumption into RemoteTabs (that each view only has one ui path to it). 

This bug is that the remoteTabs prototype takes only one ui element to show/hide if sync is enabled, and the panel view has two now(one through the menu, one through the snapped awesome screen) and they should be shown/hidden together by  setUIAccessVisible(). Right now the panel view only manipulates the former.

What this boils down to is that RemoteTabs.prototype._uiAccessElement should be an array, not a single entry.

Does that make sense? Is there some other way jimm's patches might fix this?

Comment 3

[Allison Naaktgeboren :ally]

discussed in irc, and jimm was kind enough to test on top of his (giant) patch queue and verify the issue is still around. This has led to the decision to file 869617, though not required to finish this patch.

Jimm also notes that the title string should be shorted (discussed on irc) 'Remote Tabs' is fine by me.

Comment 4

[Allison Naaktgeboren :ally]

Attachment: shorter snapped title, uses array of ui elements not a single one

Comment 5

[Jim Mathies [:jimm]]

Comment on attachment 746597 [details] [diff] [review]
shorter snapped title, uses array of ui elements not a single one

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

::: browser/metro/locales/en-US/chrome/browser.dtd
@@ +17,4 @@
>  <!ENTITY startBookmarksHeader.label       "Bookmarks">
>  <!ENTITY startHistoryHeader.label         "Recent History">
>  <!ENTITY startRemoteTabsHeader.label      "Tabs from Other Devices">
> +<!ENTITY snappedRemoteTabsHeader.label    "Remote Tabs">

probably be good to add a localization note here explaining this is a shortened version of the string above for snapped view, and that it needs to be ~ two words or less.

Comment 6

[Allison Naaktgeboren :ally]

Attachment: checkin-able version

m-i has been closed for bustage. Jimm has graciously agreed to check it in if he can.

Comment 7

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/integration/mozilla-inbound/rev/229cd16b078a

yay green trees!

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/229cd16b078a