978371 - [B2G][Browser]User unable to edit and save existing Bookmark URL address

Status: RESOLVED FIXED

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

Description

[Montoya Clemmons (MClemmons)]

When user accesses the Browser App to change an existing bookmark URL address, the Done button is disabled and the user cannot make the edit.

Prerequisites:
1) Have a website bookmarked (i.e. www.kelloggs.com)

Repro Steps:
1) Updated Buri to BuildID: 20140227040202
2) Tap Browser App and tap within URL bar to access awesome page with bookmarks tab visible
3) Tap Bookmarks tab and select a bookmarked website (i.e. www.kelloggs.com)
4) Tap yellow star icon beneath web page on device screen
5) Select Edit bookmark and tap within Address field and attempt edit of URL

Actual:
Any edits attempted and the Done button is non-selectable and user cannot change and save the address

Expected:
Any edits attempted and the Done button becomes selectable and user can change and save the address

Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140227040202
Gaia: 22d48b62df7901ad45044f66e15e7d8943884a06
Gecko: a98a1d78817f
Version: 30.0a1
Base image: v1.2-device.cfg

Notes:
1. Repro frequency: (5/5, 100%)
2. Link to failed test case: https://moztrap.mozilla.org/manage/case/1609/
3. See attached: video clip
4. Attempted with variety of websites (www.kelloggs.com, www.espn.com, etc.) with no change in outcome
5. Selection of the x button after making an attempted edit, the done button becoming not selectable, returns user to the website. Selecting the URL bar and the bookmarks tab shows no change in the Bookmark URL.

Comment 1

[Montoya Clemmons (MClemmons)]

This issue does not reproduce on the 1.3 Buri build. Following the STR in Comment 0 allows the user to edit the URL, the Done button becomes available and selectable, and the user can visualize the changed URL address when selecting from the Bookmarks tab on the awesome page. 

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140227004003
Gaia: ad504390a7a5f094f8967f80a0f29a1e6552535e
Gecko: 6bd9b70a1b6c
Version: 28.0
Base Image: v1.2-device.cfg

Comment 2

[sarsenyev]

Last Working Environmental Variables:
1.4 Environmental Variables:
Device: Buri 1.4
BuildID: 20140209160202
Gaia: f1aa7a363bc38040d0ad942e993d8641af8db752
Gecko: 7133bb431eba
Version: 30.0a1
Firmware Version: v1.2-device.cfg

First Broken Environmental Variables:
1.4 Environmental Variables:
Device: Buri 1.4
BuildID: 20140210040202
Gaia: c273bd6525f7f295539592ce74d5e6b225d53be1
Gecko: ecf20a2484b6
Version: 30.0a1
Firmware Version: v1.2-device.cfg

Last Working Gecko/First Broken Gaia: Issue DOES Reproduce
Gecko: 7133bb431eba
Gaia:  c273bd6525f7f295539592ce74d5e6b225d53be1

Last Working Gaia/First Broken Gecko: Issue does NOT reproduce
Gecko: ecf20a2484b6
Gaia:  f1aa7a363bc38040d0ad942e993d8641af8db752

Push Log: https://github.com/mozilla-b2g/gaia/compare/f1aa7a363bc38040d0ad942e993d8641af8db752...c273bd6525f7f295539592ce74d5e6b225d53be1

Comment 3

[Ben Francis [:benfrancis]]

OK this looks like a regression from bug 960813. An if statement was back to front and the disabled attribute wasn't being removed correctly.

The validation added in that patch doesn't really work brilliantly but I filed bug 979513 as a follow-up to look at that.

I've rolled a patch to fix this bug https://github.com/mozilla-b2g/gaia/pull/16848

Comment 4

[Ben Francis [:benfrancis]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/16848

Comment 5

[Dale Harvey (:daleharvey)]

Comment on attachment 8385558 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16848

So I remember specifically flagging the removeAttribute('disabled') issue in the previous review, but yeh that makes it even clearer that this needs a test, I think theres plumbing for a bookmark test in marionette already

Clearing r? for now

Comment 6

[Ben Francis [:benfrancis]]

I've added a bit more plumbing, and an Add Bookmark test. Checking Travis likes this before creating an Edit Bookmark test.

Comment 7

[Ben Francis [:benfrancis]]

Comment on attachment 8385558 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16848

Vivien, do you have a moment to review this patch while Dale is out? It's 2 lines plus integration tests.

Comment 8

[Ben Francis [:benfrancis]]

Thanks Vivien, merged into master https://github.com/mozilla-b2g/gaia/commit/76bcd49cab2c694dc3956b71361a4875a2e12f43