838041 - [Browser][User Story] Bookmark editing in bookmark tab

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Browser, defect, P3)

Description

[Peter Dolanjski [:pdol]]

UCID: Browser-047

User Story:
As a user I want to be able to edit the name or url of a bookmarked page from my "Bookmarks" tab to make them easier to use.

Comment 1

[Dylan Oliver [:doliver]]

Clearing tracking-b2g18 flag from User Story bugs. This flag is for bugs that we would take fixes for in the 1.x branch. Feature work should be officially slotted into a release instead with leo+. If this story is intended for v1.1, please nominate for leo? blocking.

Comment 2

[[e-mail]]

Attachment: add Bookmark editing/removing support in Bookmarks tab

* add listener for contextmenu in bookmark tab.
* show remove and edit options.
* use bookmark_url property to save selected bookmark url
* rewrite removeBookmark and showBookmarkEntrySheet to allow bookmark_url property from bookmark tab.
* update bookmark tab if changed

Comment 3

[Ben Francis [:benfrancis]]

Comment on attachment 720524 [details]
add Bookmark editing/removing support in Bookmarks tab

Thanks for the patch, but I've noticed a few problems when doing some brief initial testing.

Firstly I keep seeing the following error in the console when using this feature:

E/GeckoConsole(  774): [JavaScript Error: "TypeError: Places.getBookmark(...) is undefined" {file: "app://browser.gaiamobile.org/js/browser.js" line: 970}]

Secondly, sometimes the "Add to home screen" option disappears from the bookmark menu and won't come back.

Thirdly, this context menu appears when long pressing on entries in the history and top sites tabs which gives confusing results.

Could you take a look at these issues before I test further or finish my review?

Also, might there be a way to pass the URL to the method that displays the menu rather than storing it in a temporary property on the Browser object?

Is this a 1.1 feature btw? It's not marked in any way to say that it is, how do we know?

Thanks

Comment 4

[Dylan Oliver [:doliver]]

(In reply to Ben Francis [:benfrancis] from comment #3)
> Is this a 1.1 feature btw? It's not marked in any way to say that it is, how
> do we know?
> 

The related bug 838040 is a v1.1 feature. This is also a 1.1 story but for the most part only the P1 stories are leo+. We'd take this one if it also solves the blocking bug, but 838040 is the priority.

Comment 5

[[e-mail]]

Thanks for review,

1st issue is because of miss-binding, fixed.

2nd, should "Add to home screen" option appear when hit a 'stared' bookmark? (I didn't find this option in current behavior when I hit the 'star' in browser tab). Maybe we could clearify it.

3rd, I add an 'current_tab' option for drawAwesomescreenListItem. current_tab is used for distinguish the current tab is bookmark. Now other tabs wont bind with long press menus.

Comment 6

[[e-mail]]

Attachment: fix miss-binding and bound contextmenu only in bookmark tab

Comment 7

[Ben Francis [:benfrancis]]

Comment on attachment 722095 [details]
fix miss-binding and bound contextmenu only in bookmark tab

Please see comments on GitHub

Comment 8

[[e-mail]]

Made a candidate pull request https://github.com/mozilla-b2g/gaia/pull/8606

I found after 'reset-gaia' then open browser.app immediately, there's a high chance to get default mcc/mnc settings as 0.

but navigator.mozMobileConnection.iccInfo.mcc/mnc can always get the correct value.

Comment 9

[[e-mail]]

sorry Comment 8 is put in wrong issue # (should be reply to 835350)

Comment 10

[[e-mail]]

Attachment: Support Edit bookmark in bookmark tab

Follow new approach to implement bookmark edit in bookmark tab.
Need advice about updatebookMark behavior (see in github pullreqest comment).

Comment 11

[Ben Francis [:benfrancis]]

Comment on attachment 731762 [details]
Support Edit bookmark in bookmark tab

updateBookmark won't take effect synchronously, you'll need to wait for the callback to say it's been completed, then update the view accordingly.

I really don't like the use of "dataset.url" in this method and it's getting a little messy. I'd really like to refactor this bit of code myself if possible so don't worry if you don't get to it.

Comment 12

[:CSchmoeckel]

Enabled Test Case #1890: Able to edit web site bookmarks
For version 1.1.0 testing.

Comment 14

[ffos-browser]

A Pivotal Tracker story has been created for this Bug: http://www.pivotaltracker.com/story/show/53548961

Comment 15

[clwu]

we need to fixed it on V1.1.
Thanks.

Comment 16

[Wayne Chang [:wchang]]

Triage - not blocking for triaging partners as this is a feature request for 1.1.

Comment 17

[Viga]

Any update on this bug? Are we going to fix it on v1.1, or not?

Comment 18

[Ben Francis [:benfrancis]]

Ian, Karen, I think this is sprint ready because we have specs here http://people.mozilla.com/~lco/FX_B2G/Release_1_Specs/R1_Bookmarks_v2.pdf

Comment 19

[Karen Rudnitski [:kar]]

Ian, Francis - these specs look fine by me. Please confirm they're the latest (attached in C18) and I'll flag this for sprint-ready.

Comment 20

[Peter Dolanjski [:pdol]]

Given the Browser OS work, moving to WONTFIX.