[Contacts]The delete function in contact details view does not work.

VERIFIED FIXED in mozilla37

Status

()

defect
P2
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: lulu.tian, Assigned: TYLin)

Tracking

({regression})

Trunk
mozilla37
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 unaffected, b2g-v2.2 verified)

Details

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Posted image screenshot
[1.Description]:
When you delete a contact from contact details view, the delete button colour will turn faint, but it can not be deleted successfully.
See attachment(logcat_0955 and 0955.png)
Found time:09:55

[2.Testing Steps]: 
1. Launch Contacts.
2. Tap a contact to check details.
3. Tap edit icon at right top.
4. Slide down and tap delete button. 

[3.Expected Result]: 
4. The contact should be delete.

[4.Actual Result]: 
4. There is no response when you tap delete button.

[5.Reproduction build]: 
Gaia-Rev        86d83f4b4111ca45ebc92ca779348cc966f43cff
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f8432250efb7
Build-ID        20141023160201
Version         32.0

[6.Reproduction Frequency]: 
occasionally Recurrence,3/10
(Reporter)

Comment 1

5 years ago
Posted file logcat
(Reporter)

Updated

5 years ago
Summary: [MGSEI][Flame][v2.0][Contacts]The delete function in contact details view does not work. → [Flame][v2.0][Contacts]The delete function in contact details view does not work.
(Reporter)

Updated

4 years ago
Summary: [Flame][v2.0][Contacts]The delete function in contact details view does not work. → [Contacts]The delete function in contact details view does not work.
(Reporter)

Comment 2

4 years ago
When long press at "Delete contact" button, sometimes, there is no response.
See attachment: logcat_v2.2_1809.txt and video.MP4
(Reporter)

Comment 3

4 years ago
Posted video video.MP4
Nominating for 2.2 as it's a mayor feature broken
blocking-b2g: --- → 2.2?
Just tried on master and cannot reproduce, Johan can you take a look?
Flags: needinfo?(jlorenzo)
I triggered that bug on the first try by long pressing the delete button like Sue did on the video. I haven't succeeded to reproduce the problem on 2.1. We'll need a regression window here.
Flags: needinfo?(jlorenzo)
Yes the key to repro is to "long press" on the Delete contact button. 2.2 is affected and 2.1 is unaffected. Working on getting the window now.
QA Contact: pcheng
mozilla-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20141102230636
Gaia: c2c55870de22d596de4f41612c0b44090f90ebad
Gecko: 14f544cf024a
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20141103000037
Gaia: c2c55870de22d596de4f41612c0b44090f90ebad
Gecko: 667a5bff1599
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=14f544cf024a&tochange=667a5bff1599
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Not really sure what could be the cause of this issue, but we did verify the Last working build with addition checks.  

Johan do you have any opinions on the pushlog on comment 8?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado) → needinfo?(jlorenzo)
Triage: Visible regression in 2.2 which might lead a user to think that the functionality is broken.
blocking-b2g: 2.2? → 2.2+
This might be related to bug 1029943, what do you think Ting-Yu?
Flags: needinfo?(jlorenzo) → needinfo?(tlin)
(Assignee)

Comment 12

4 years ago
Yes. It relates to bug 1029943. SelectionCarets::SelectWord() consumes long press event even if it doesn't  select a word. SelectionCarets shouldn't consume it in this case.
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Flags: needinfo?(tlin)
Component: Gaia::Contacts → Selection
Product: Firefox OS → Core
Version: unspecified → Trunk
(Assignee)

Updated

4 years ago
Comment on attachment 8540597 [details] [diff] [review]
Do not consume NS_MOUSE_MOZLONGTAP if SelectWord failed. f=mtseng, r=roc (v1)

Nice!
Attachment #8540597 - Flags: feedback?(mtseng) → feedback+
(Assignee)

Updated

4 years ago
Attachment #8540597 - Flags: review?(roc)
Comment on attachment 8541106 [details] [diff] [review]
Do not consume NS_MOUSE_MOZLONGTAP if SelectWord failed. f=mtseng, r=roc (v2)

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

::: layout/base/SelectionCarets.cpp
@@ +554,2 @@
>    if (!mPresShell) {
> +    return succeeded;

For success results, you should keep using nsresult, but you should return a failure code here since it failed.
Attachment #8541106 - Flags: review?(roc) → review-
(Assignee)

Comment 17

4 years ago
Thanks, roc. Addressed your comments by returning nsresult.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b4ee66696e3
Attachment #8541106 - Attachment is obsolete: true
Attachment #8541377 - Flags: feedback?(mtseng)
Comment on attachment 8541377 [details] [diff] [review]
Do not consume NS_MOUSE_MOZLONGTAP if SelectWord failed. f=mtseng, r=roc (v3)

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

::: layout/base/SelectionCarets.cpp
@@ +264,5 @@
>      if (!mVisible) {
>        SELECTIONCARETS_LOG("SelectWord from APZ");
> +      nsresult wordSelected = SelectWord();
> +
> +      if (NS_FAILED(wordSelected)) {

Should we call SelectWord() directly here? Looks like 'wordSelected' never be used.

@@ +1210,5 @@
>  
>    SELECTIONCARETS_LOG_STATIC("SelectWord from non-APZ");
> +  nsresult wordSelected = self->SelectWord();
> +
> +  if (NS_FAILED(wordSelected)) {

Ditto.
Attachment #8541377 - Flags: feedback?(mtseng) → feedback+
(Assignee)

Comment 19

4 years ago
Comment on attachment 8541377 [details] [diff] [review]
Do not consume NS_MOUSE_MOZLONGTAP if SelectWord failed. f=mtseng, r=roc (v3)

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

(In reply to Morris Tseng [:mtseng] from comment #18)
> Comment on attachment 8541377 [details] [diff] [review]
> Do not consume NS_MOUSE_MOZLONGTAP if SelectWord failed. f=mtseng, r=roc (v3)
> 
> Review of attachment 8541377 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/base/SelectionCarets.cpp
> @@ +264,5 @@
> >      if (!mVisible) {
> >        SELECTIONCARETS_LOG("SelectWord from APZ");
> > +      nsresult wordSelected = SelectWord();
> > +
> > +      if (NS_FAILED(wordSelected)) {
> 
> Should we call SelectWord() directly here? Looks like 'wordSelected' never
> be used.

I would like to make the return value meaningful and explicit. Thanks.
Attachment #8541377 - Flags: review?(roc)

Updated

4 years ago
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/597b1585ab72
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
(Reporter)

Comment 23

4 years ago
Posted video verify_v2.2.MP4
This issue is verified not happen in latest build of Flame 2.2

See attachment:verify_v2.2.MP4
Flame 2.2 build:
Gaia-Rev        69ac77cfa938fae2763ac426a80ca6e5feb6ad25
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/33781a3a5201
Build-ID        20150107010216
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150107.044401
FW-Date         Wed Jan  7 04:44:12 EST 2015
Bootloader      L1TC000118D0
(Reporter)

Updated

4 years ago
(Reporter)

Comment 24

4 years ago
This issue still exist on Woodduck 2.0M and Flame 2.0

Flame 2.0 build:
Gaia-Rev        f76014fd2c7528493b90d759c68ec3070233d094
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/53ff92e647a0
Build-ID        20150107000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150107.033038
FW-Date         Wed Jan  7 03:30:49 EST 2015
Bootloader      L1TC000118D0

Woodduck build:
Gaia-Rev        
06b544f8a9ee03754dd3f5020f279cffa7a75804
Gecko-Rev       6b1e24a1cba1d5542bebb503ce05a1ec61ceb7d3
Build-ID        20150108050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1420664769
FW-Date         Thu Jan  8 05:06:39 CST 2015
I think we should ask for an approval b2g32 and b2g34. Ting?
Flags: needinfo?(tlin)
(Reporter)

Updated

4 years ago
Group: woodduck-confidential
(Assignee)

Comment 26

4 years ago
Uplifting my patch to v2.0 and v2.1 branch will not help.  Since the copy & paste feature (TouchCaret and SelectionCarets) does not being enabled on those branches, the code does not affect the behavior. 

Without my patch, nearly all the buttons do not work if the user long press on them such as the following steps in description. 
2. Tap a contact to check details.
3. Tap edit icon at right top.

If they are working on v2.0 or v2.1, it might be other issue.
Flags: needinfo?(tlin)
I checked on the same build as you on 2.0, I was able to repro.

The difference between 2.2 is that the button keep working if you press again after long pressing on it. It wasn't the case when the text got selected in 2.2.

Sue, as the issue has different causes in 2.0 and 2.2, would you mind opening a new bug for 2.0 with a logcat on this branch?

Clearing the woodduck-confidential group as the bug occurred on the Flame.
Group: woodduck-confidential
Status: RESOLVED → VERIFIED
Flags: needinfo?(lulu.tian)
(Reporter)

Updated

4 years ago
See Also: → 1119668
(Reporter)

Comment 28

4 years ago
(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #27)
> I checked on the same build as you on 2.0, I was able to repro.
> 
> The difference between 2.2 is that the button keep working if you press
> again after long pressing on it. It wasn't the case when the text got
> selected in 2.2.
> 
> Sue, as the issue has different causes in 2.0 and 2.2, would you mind
> opening a new bug for 2.0 with a logcat on this branch?
> 
> Clearing the woodduck-confidential group as the bug occurred on the Flame.

Hi Johan,
I have opened a new bug for Flame 2.0, see Bug 1119668
Flags: needinfo?(lulu.tian)
You need to log in before you can comment on or make changes to this bug.