1114509 - [Flame][Homescreen] The text edit botton is still displayed in homescreen after exiting homescreen edit mode.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P2)

Description

[Sue]

Attachment: logcat_1614.txt

[1.Description]:
[Flame][V2.2][Homescreen] The text edit botton is still displayed in homescreen after exiting homescreen edit mode.
Found time:16:13
See attachment:1613.MP4 and logcat_1613.txt

[2.Testing Steps]: 
1. Enter in Home Screen Edit Mode by long-pressing an icon. 
2. Tap on a bookmark. 
3. Long tap "Done" button
**Pops up edit box and button.
4. Tap "Done" button

[3.Expected Result]: 
4.The homescreen is displayed normally after exiting homescreen edit mode.

[4.Actual Result]: 
4.The edit button is still displayed in homescreen after exiting homescreen edit mode.

[5.Reproduction build]: 
Gaia-Rev        ca6e91e09ef3ab417a0f6b6d6668d43597d85700
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/7b33ee7fd162
Build-ID        20141221040207
Version         37.0a1

[6.Reproduction Frequency]: 
Always Recurrence,5/5
TCID: 11858

Comment 1

[Sue]

Attachment: video

Comment 2

[Eric Chang [:ericcc] [:echang]]

Attachment: 2014-12-24-11-44-34.png

Hi Gerry, Please take a look at this, thanks.

Comment 3

[Gerry Chang [:gchang]]

NI developer for this issue.

Comment 4

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

According to bug 1092944, we should let most of dom unselectable when copy-paste is enabled. As I know, homescreen have no area for copy-paste?

could you kindly check my commit? Thanks.

Comment 5

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8541379 [details] [review]
PR to master

Hi Kevin,
could you review this patch?
The button should not be selectable.

Comment 6

[Kevin Grandon :kgrandon]

George - I don't really like the idea of introducing a * selector here. Can we instead update the gaia-header web component to do this?

Comment 7

[Kevin Grandon :kgrandon]

Also adding Wilson here. Wilson - what are your thoughts on adding this to the web component itself?

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8541379 [details] [review]
PR to master

Please flag me again after addressing questions. Thanks!

Comment 9

[Wilson Page [:wilsonpage]]

(In reply to Kevin Grandon :kgrandon from comment #7)
> Also adding Wilson here. Wilson - what are your thoughts on adding this to
> the web component itself?

I'm happy to add a rule to prevent user-selection of button contents to gaia-header. I'd rather kill the 'Done' button though ;) Feels very 'old-school'.

Comment 10

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8541379 [details] [review]
PR to master

Thanks Kevin, I should add the rule to gaia-header. Please kindly check again.

Comment 11

[Kevin Grandon :kgrandon]

Comment on attachment 8541379 [details] [review]
PR to master

I think the patch looks good to me, but we need to make it against the upstream gaia-header repository first, then bring those changes into gaia with bower.

George - can you make the pull request against this repo here instead: https://github.com/gaia-components/gaia-header?

Otherwise, let me know if you want me to do it. Thanks!

Comment 12

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to gaia-header

Hi Kevin,
update my patch.
After your reviewing, I guess I also need to update the bower.json version to 0.5.6 if needed and then submit a gaia pr, right?

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8544410 [details] [review]
PR to gaia-header

https://github.com/gaia-components/gaia-header/pull/18

Comment 14

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to gaia-header

Comment 15

[Wilson Page [:wilsonpage]]

Comment on attachment 8544413 [details] [review]
PR  to gaia-header

LANDED https://github.com/gaia-components/gaia-header/commit/25212c82672cc8c2e80b4f23db49e51f9747f2d0

Comment 16

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Update gaia-header to v0.5.7

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 8544528 [details]
pull-request (master)

Needs rebasing, but seems good to me.

Comment 18

[(Inactive after June) George Duan [:gduan] [:喬智]]

Assign this bug to Wilson.

please kindly merge your commit, thanks for your help!

Comment 19

[Wilson Page [:wilsonpage]]

Oops soz, thought this had landed.

Comment 20

[Wilson Page [:wilsonpage]]

Rebased and pushed again, waiting on greenness.

Comment 21

[Wilson Page [:wilsonpage]]

Lot's of red from unrelated test failures preventing me from landing this.

Comment 22

[Wilson Page [:wilsonpage]]

Comment on attachment 8544528 [details]
pull-request (master)

LANDED https://github.com/mozilla-b2g/gaia/commit/4255c693940f6676a06449f0337e3d5f9d3cb2bf

Comment 23

[Eric Chang [:ericcc] [:echang]]

Copy/paste is a feature for 2.2, set 2.2?

Comment 24

[Sue]

Attachment: logcat_v2.2_1822.txt

Does it fixed on Flame v2.2? I can repro this issue on latest build of Flame v2.2. The text edit botton is still displayed in homescreen after exiting homescreen edit mode.
Found time:18:22
See attachment:logcat_v2.2_1822.txt

Flame 2.2 build:
Gaia-Rev        f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8067c111ddff
Build-ID        20150118002501
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150118.035516
FW-Date         Sun Jan 18 03:55:27 EST 2015
Bootloader      L1TC000118D0

Comment 25

[Kevin Grandon :kgrandon]

The 2.2 flag is still set to affected, so I assume that we need to request uplift here.

Comment 26

[Kevin Grandon :kgrandon]

Comment on attachment 8544528 [details]
pull-request (master)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Copy/paste feature implementation.
[User impact] if declined: Poor experience when using home screen and seeing the edit menu pop up.
[Testing completed]: Manual testing.
[Risk to taking this patch] (and alternatives if risky): Low risk, mainly an update to the gaia-header component. It touches many apps, but is a small change.
[String changes made]: None.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/5f951ad4f8d66dbde5e94542bef893bb53750a83

Comment 28

[Yeojin Chung [:YeojinC]]

This issue STILL reproduces on Flame Master and 2.2.

Result: The carats and copy/paste bubble stay on the screen after exiting the edit mode.
 
Device: Flame 2.2 (319mb, full flash)
BuildID: 20150122002808
Gaia: e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko: 4a90da67661e
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame Master (319mb, full flash)
Build ID: 20150122010203
Gaia: 917b6c36717fddc6e71ffc1ec249633c8044c93c
Gecko: 34e2d2bd7ec4
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 29

[Sue]

Attachment: 2015-01-22-09-22-24.png

According to comment 28, this issue was verified fail on Flame 2.2, the carats bubble stay on the screen after exiting the edit mode. Reopen it.
See attachment:2015-01-22-09-22-24.png

Comment 30

[GitHub Autolander Bot]

Attachment: [gaia] wilsonpage:1114509 > mozilla-b2g:master

Comment 31

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

The reason the original gaia-header patch didin't fix the bug id because the patch was applied to dist/gaia-header.js not gaia-header.js. This is something I should have caught in review, sorry about this.

I've reapplied the fix to gaia-header. This patch updates gaia-header to the latest v0.6.5.

Comment 32

[Julien Wajsberg [:julienw]]

I believe it was incorrectly set to 2.0+; please set it to 2.2+ instead :)

Comment 33

[Julien Wajsberg [:julienw]]

This landed in gaia-header in 2 commits:
https://github.com/gaia-components/gaia-header/commit/08c6f18b5d56213b1131e071841e4eb355014a32
https://github.com/gaia-components/gaia-header/commit/d9cb848c2b3d7e71e6cb420af81fe46382b3b81c

Comment 34

[Julien Wajsberg [:julienw]]

I'm not sure I can reproduce properly the original issue, and the header is still selectable with the patch.

I'll sync with Wilson today.

Comment 35

[Wilson Page [:wilsonpage]]

UPDATE

We found that the 'bookmarks' app wasn't using <gaia-header> but old headers.css. This patch has been amended to include this update.

Comment 36

[bhavana bajaj [:bajaj]]

(In reply to Wilson Page [:wilsonpage] (away 'til 23rd Feb) from comment #35)
> UPDATE
> 
> We found that the 'bookmarks' app wasn't using <gaia-header> but old
> headers.css. This patch has been amended to include this update.

Marking this resolved fixed in this case and requesting verification again.

Comment 37

[Julien Wajsberg [:julienw]]

Comment on attachment 8563330 [details] [review]
pull-request (master)

I mainly check behavior correctness, and this looks good, except for an issue in the Music application (more info on Github).

I think we should have a review from the peers for these applications (music, calendar, verticalhome, bookmark) because they know better than me what we need to test.

Comment 38

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame 2.2/3.0
STR:
1. Enter in Home Screen Edit Mode by long-pressing an icon. 
2. Long tap "Done" button
**It will exiting homescreen edit mode and homescreen displayed normally.
See attachment:verify_video.MP4
Rate:0/5

Flame 2.2 build:
Build ID               20150225002505
Gaia Revision          ca64f2fe145909f31af266b1730874051ba76c78
Gaia Date              2015-02-24 22:06:53
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/16804008c29f
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150225.041814
Firmware Date          Wed Feb 25 04:18:25 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 build:
Build ID               20150225010244
Gaia Revision          f6bfd854fe4746f21bc006eac145365e85f98808
Gaia Date              2015-02-24 21:10:44
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0a8b3b67715a
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150225.043702
Firmware Date          Wed Feb 25 04:37:14 EST 2015
Bootloader             L1TC00011880