939092 - [l10n] Defect - Bookmarks are not shared between profiles

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Browser, defect, P2)

Description

[Manuela Muntean [Away]]

- on Win 8 64-bit, with latest oak build (build ID: 20131114040200)

STR

1. Install latest oak build from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-oak/ and make it your default browser

2. Open the browser in the Desktop mode, bookmark some pages and then launch it in Metro mode.

3. Open the browser in the Metro mode, bookmark some pages and then launch it in Desktop mode.

Expected results: both after step 2 and 3, the bookmarked pages should appear as bookmarked in both modes

Actual results: both after step 2 and 3, the bookmarked pages don't appear as bookmarked in both modes

Comment 1

[Brian R. Bondy [:bbondy]]

Hi Marina,

Candidate for next pick.

p=3

Comment 2

[Brian R. Bondy [:bbondy]]

Subtracted 3 from contingency for this

Comment 3

[Jim Mathies [:jimm]]

This shouldn't block since this is by design. Our metro bookmarks are stored separately and we currently don't expose desktop bookmarks through the metro ui. This is waiting on hierarchical bookmark view support which got booted to a future release.

Comment 4

[Brian R. Bondy [:bbondy]]

Couldn't we just use the Desktop file and for any bookmark created in metro, it would be top level on Desktop. And for anything in a folder in Desktop we don't show it in Metro?

Comment 5

[Jim Mathies [:jimm]]

(In reply to Brian R. Bondy [:bbondy] from comment #4)
> Couldn't we just use the Desktop file and for any bookmark created in metro,
> it would be top level on Desktop. And for anything in a folder in Desktop we
> don't show it in Metro?

ni'ing yaun since this is really a ux decision vs. an engineering one. Switching to desktop's top level folder shouldn't be too hard to do. We could also add a "Metro" sub-folder to desktop bookmarks and put metro bookmarks in that.

Comment 6

[Brian R. Bondy [:bbondy]]

Ya good point, Yuan we'd ideally want a temporary solution that is not a huge undertaking (like adding hierarchical bookmark support to Metro), which would allow a user to find his bookmarks in the other environment.  I think both my and Jim's suggestions would work but I'd love to hear what you think is best here.

Comment 7

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Given the efforts and complexity of displaying hierarchical bookmarks from desktop, I think we should make bookmarks only searchable via auto-complete, and wait for v2 to have a complete view to browse. 

On the desktop side, I am leaning towards have a dedicated "Metro" folder for bookmarks created from metro. Since it's a single flat list, it should be pretty straightforward for users.

Thoughts?

Comment 8

[Marina Samuel [:emtwo]]

I like the Metro subfolder idea. I wish we could maybe somehow show a subset of the desktop bookmarks on metro so the user knows they are there and that if they start typing, we'll autocomplete. But either way, having them in autocomplete is better than nothing.

Comment 9

[Brian R. Bondy [:bbondy]]

It would be better to get this in before 28 is uplifted so changing to block28 for now.
Let's do the metro subfolder idea for now and we'll revisit a better solution later.

Comment 10

[Brian R. Bondy [:bbondy]]

Would be nice to get it in for this iteration if possible too.

Comment 11

[Jim Mathies [:jimm]]

We'll need a suitable name for the sub folder too. "Firefox Touch" or would look good. :)

Comment 12

[Marco Mucci [:MarcoM]]

Hey Marina, do you want this defect added to this iteration or the next one?

Comment 13

[Marina Samuel [:emtwo]]

Hey Marco, I'd like to add it to this iteration

Comment 14

[Marina Samuel [:emtwo]]

Attachment: wip: Firefox Touch bookmarks folder on desktop

In order to test this, I needed to set the pref "browser.bookmarks.restore_default_bookmarks" to 'true' and restart the browser so that this new folder is added.

Comment 15

[Marina Samuel [:emtwo]]

Attachment: firefoxtouch_bookmarks.png

A screenshot of where the Firefox Touch folder is

Comment 16

[Brian R. Bondy [:bbondy]]

Comment on attachment 8334884 [details] [diff] [review]
wip: Firefox Touch bookmarks folder on desktop

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

That looks good to me in general but we need another patch for the bookmarks to go into there.

Comment 17

[Marina Samuel [:emtwo]]

The bookmarks that exist and are added do go in there because the URI used is the folder that metro places bookmarks in: PlacesUtils.annotations.getItemsWithAnnotation('metro/bookmarksRoot', {})[0]

Comment 18

[Brian R. Bondy [:bbondy]]

oh nice, ok I'll give it a spin

Comment 19

[Brian R. Bondy [:bbondy]]

You mentioned WIP btw, what do you know that is remaining?

Comment 20

[Marina Samuel [:emtwo]]

Ah sorry I forgot to mention. I was thinking a user who already has firefox installed won't see their bookmarks on desktop unless we go down the code path that sets 'browser.bookmarks.restore_default_bookmarks' to true. I don't think we want to restore all default bookmarks just to have the folder show up. So we need a way such that the folder is added on upgrade or something like that. Does that sound right?

Comment 21

[Marina Samuel [:emtwo]]

Actually, I just found a comment here explaining how to properly do this :)

http://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1589

Comment 22

[Brian R. Bondy [:bbondy]]

Awesomeness, I haven't had a chance to look into it and I don't know the code well, so that's great :)

Comment 23

[Marina Samuel [:emtwo]]

Attachment: v2: Firefox Touch bookmarks folder on desktop

Change version for smart bookmarks so that the metro folder appears without a pref.

Comment 28

[Marina Samuel [:emtwo]]

Also worth noting that if someone would like to view some bookmarks from desktop on metro, they can put them in their "Firefox Touch" folder and they will be visible in metro.

Comment 29

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/projects/oak/rev/20ca666faf0e

Bug 935099 will track the landing on m-c.
When it lands on m-c a new comment will be added here as well with the m-c changeset.
This is being done so we can still use scrumbugs efficiently. 
See bug 935099 for further details.

Comment 30

[Brian R. Bondy [:bbondy]]

Looks like this is causing xpcshell test failures on oak, could you take a look?
Both of the test failures are highest priority because they are blocking the uplift to oak. Thanks!
https://hg.mozilla.org/projects/oak/rev/20ca666faf0e

Comment 31

[Marina Samuel [:emtwo]]

Attachment: xpcshell-test-fix

head_bookmarks.js needed updated constants. The xpcshell-test for browser/components/places now passes locally

Comment 32

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/projects/oak/rev/4038d243ca65

Comment 33

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/fx-team/rev/88b60ad4d8c6
https://hg.mozilla.org/integration/fx-team/rev/0f6484c13cd2

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88b60ad4d8c6
https://hg.mozilla.org/mozilla-central/rev/0f6484c13cd2

Comment 35

[pal-moz]

bookmarks in Touch folder should not be same as in Recently Bookmarked folder ?

http://forums.mozillazine.org/viewtopic.php?p=13222175#p13222175

Comment 36

[M** A****]

I think it's slightly strange that if you delete bookmarks from the touch folder that they are also permanently deleted from their original location with no warning given.
Was any thought given to this problem?

Comment 37

[M** A****]

Also seeing as this is a Metro Bug why is it being installed on Win7?

Comment 38

[Manuela Muntean [Away]]

Tested this for iteration #19, on Win 8 64-bit, with latest Nightly from 2013-12-01:

1) if I open the browser in the Metro mode, bookmark some pages and then launch it in Desktop mode, the pages are bookmarked here too, and appear in the Touch folder

2) after opening the browser in the Desktop mode, bookmarking some pages and then launching it in Metro mode, the pages are indeed bookmarked, but they don't appear under the Bookmarks list from the Start Page; is this expected?  (also bookmarked pages appear in the Touch folder )

Comment 39

[Brian R. Bondy [:bbondy]]

(In reply to Manuela Muntean [:Manuela] [QA] from comment #38)
> Tested this for iteration #19, on Win 8 64-bit, with latest Nightly from
> 2013-12-01:
> 
> 1) if I open the browser in the Metro mode, bookmark some pages and then
> launch it in Desktop mode, the pages are bookmarked here too, and appear in
> the Touch folder

That sounds correct.

> 
> 2) after opening the browser in the Desktop mode, bookmarking some pages and
> then launching it in Metro mode, the pages are indeed bookmarked, but they
> don't appear under the Bookmarks list from the Start Page; is this expected?
> (also bookmarked pages appear in the Touch folder )


This is expected, but I'm going to needinfo yuan because I think maybe we want the bookmarks to NOT show as bookmakred from the navigation appbar if they aren't in the metro annotated bookmarks folder. If that's the case we should post a new bug for this.

Comment 40

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

That's by design. Since on Metro v1, there is no bookmark folders/lists supported, it's tricky to accommodate a large number of bookmarks. 
Users can still access the desktop bookmarks from auto-complete screen when there are matched results.

Comment 41

[Marina Samuel [:emtwo]]

Comment on attachment 8334885 [details]
firefoxtouch_bookmarks.png

Hi Yuan, could we please also get a review for the bookmarks folder verbage? Do we also want to use "Windows 8 Touch" here instead of "Firefox Touch"? Thanks!

Comment 42

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Marina Samuel [:emtwo] from comment #41)
> Comment on attachment 8334885 [details]
> firefoxtouch_bookmarks.png
> 
> Hi Yuan, could we please also get a review for the bookmarks folder verbage?
> Do we also want to use "Windows 8 Touch" here instead of "Firefox Touch"?
> Thanks!

I think "Windows 8 Touch" would be appropriate here.

Comment 43

[Marina Samuel [:emtwo]]

Attachment: v1: Update Metro folder text

Comment 44

[Marina Samuel [:emtwo]]

https://hg.mozilla.org/integration/fx-team/rev/4a33910c9c87

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4a33910c9c87

Comment 46

[pal-moz]

(In reply to pal-moz from comment #35)
> bookmarks in Touch folder should not be same as in Recently Bookmarked
> folder ?
> 
> http://forums.mozillazine.org/viewtopic.php?p=13222175#p13222175

no answer ?

Comment 47

[Manuela Muntean [Away]]

Attachment: screenshot.png

Retested this with latest en-US Nightly build from 2013-12-04 (with Australis) on Win 8 64-bit and got the same results:

> 1) if I open the browser in the Metro mode, bookmark some pages and then
> launch it in Desktop mode, the pages are bookmarked here too, and appear in
> the Touch folder
> 
> 2) after opening the browser in the Desktop mode, bookmarking some pages and
> then launching it in Metro mode, the pages are indeed bookmarked, but they
> don't appear under the Bookmarks list from the Start Page; is this expected?
> (also bookmarked pages appear in the Touch folder )

The only difference is that the switch button from Desktop mode to Metro mode is now called "Windows 8 Touch" (please see attached screenshot for details).

Comment 48

[Manuela Muntean [Away]]

Marking this as verified, based on comment 39 and comment 47.

Comment 49

[Dominic]

After I sync with another device (which runs W7), the sync'd bookmarks appear in the Desktop mode but not in Windows 8 Touch mode. This makes W8T mode pretty useless for me.