Closed Bug 1078978 Opened 9 years ago Closed 9 years ago

When we checked all contacts then we searched a contact,the contact display uncheck.

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Contacts
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
VERIFIED FIXED
Project Flags:
blocking-b2g 2.0M+
Tracking Flags:
Tracking Status
b2g-v2.0 --- affected
b2g-v2.0M --- verified
b2g-v2.1 --- affected
b2g-v2.2 --- verified

People

(Reporter: selee, Assigned: selee)

References

Details

Attachments

(2 files)

Pull request for master
46 bytes, text/x-github-pull-request
sergi
: review+
Details | Review
Woodduck verify video
1.30 MB, video/mp4
Details 
+++ This bug was initially created as a clone of Bug #1070456 +++

[step]
1. Launch Contacts
2. Tap Setting icon->tap "Export Contacts"->select SIM1
3. Check two contacts
4. Input a contact name to "Search"box

[Expected Result]
The contact should check.

[Error]
The contact does not check.
Hello Etienne,

Please help to review my patch.
Thank you.
Attachment #8500814 - Flags: review?(etienne)
Comment on attachment 8500814 [details] [review]
Pull request for master

redirecting...
Attachment #8500814 - Flags: review?(etienne) → review?(sergi.mansilla)
Comment on attachment 8500814 [details] [review]
Pull request for master

Hi Sean,

Thanks for your patch. I gave it an r- for two reasons (both of which are not your fault at all, but of the previous code):

- I find the code confusing and this is a good opportunity to make it better. That RegExp is giving me an eyesore.

- I believe there is a project-wide decision of not using `innerHTML` to manipulate the DOM due to security concerns; it would be great if you can use normal DOM manipulation. This code could use a DocumentFragment to build the final result, for example.

Cheers!
Attachment #8500814 - Flags: review?(sergi.mansilla) → review-
Flags: needinfo?(selee)
Hi Sergi,

Sorry to interrupt your discussion but I would suggest keeping the patch of this bug as simple as possible. This bug is marked "2.0M?" so there will be much chance that we need to uplift it to v2.0 and v2.0m branch. To replace RegExp and innerHTML seems to be non-trivial and risky for uplifting. Maybe we can file another bug for refining this part of code. What do you think?
Flags: needinfo?(sergi.mansilla)
Comment on attachment 8500814 [details] [review]
Pull request for master

Hi Luke,

Yes, you are right. In this case it is an r+ for me if the follow-up bug is created.

Thanks!
Attachment #8500814 - Flags: review- → review+
Flags: needinfo?(sergi.mansilla)
Blocks: 1080451
Hi Sergi,

Thanks for your comments.
I've created a new bug 1080451 to refine replaceWithHighlight function as your suggestions.
Flags: needinfo?(selee)
Chris, can you please verify that this issue is fixed?
Flags: needinfo?(ckreinbring)
Keywords: verifyme
Fix verified on Flame 2.2 nightly.
Actual result: When the user searches for a contact that is already selected for exporting, the checkbox next to the contact name will be checked.

BuildID: 20141015040201
Gaia: 5f1f0960ae9d22acf2a324ad37a48174d6df87f6
Gecko: 62f0b771583c
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Platform Version: 36.0a1
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

--------------------------------------------------------------------------------------------------------

The bug repros on Flame 2.1 and Flame 2.0, both nightly.
Actual result: When the user searches for a contact that is already selected for exporting, the checkbox next to the contact name will be unchecked.

Flame 2.1
BuildID: 20141015001201
Gaia: 379ea4c9dd6d3f8ca2f79ce59c15f6afe6e557c3
Gecko: 4853208cb48a
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Platform Version: 34.0
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0
BuildID: 20141015000206
Gaia: c6c6116ca225c2c934220ae6867e5a3256d65e00
Gecko: 24a2aa6bf1c4
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Platofrm Version: 32.0
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
Flags: needinfo?(ckreinbring) → needinfo?(ktucker)
Leaving verifyme for 2.1 vefication once =fix has been uplifted.
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
HI Kai-Zhen,
2.0M+ Thanks!
Flags: needinfo?(kli)
Blocks: Woodduck
blocking-b2g: 2.0M? → 2.0M+
Sean, Uplift into v2.0m got conflicts. Could you provide a patch for v2.0m? Thanks!
Flags: needinfo?(kli) → needinfo?(selee)
Hi Seinlin,

The bug is fixed in v2.0m already. (I tested this in Woodduck again.)
I think we don't need to uplift this. Thank you.
Flags: needinfo?(selee)
Attached video Woodduck verify video — 
This problem is verified not to happen on Woodduck2.0
See attachment: Woodduck_verify_video.mp4
Occurrence rate: 0/10

Woodduck 2.0 build:
Gaia-Rev        add38992bbfb2bafca52ac1ce7f6231ac702675f
Gecko-Rev       8d951d9c4988e218ec08c6118a35c7faeb70af49
Build-ID        20141225143517
Version         32.0
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.