929043 - [Contacts] Contacts .remove function used broken due to webidl

Status: RESOLVED FIXED

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

Description

[Francisco Jordano [:arcturus] [:francisco]]

+++ This bug was initially created as a clone of Bug #928875 +++

It looks like webidl also broke then merge:

E/GeckoConsole(  417): [JavaScript Error: "TypeError: Argument 1 of ContactManager.remove does not implement interface mozContact." {file: "app://communications.gaiamobile.org/contacts/js/contacts_merger.js" line: 213}]

Comment 1

[Francisco Jordano [:arcturus] [:francisco]]

Use this but to fix all the instances of the .remove problem with the webidl drama

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to PR 12985

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 819862 [details]
Pointer to PR 12985

I think we might be make more mozContact objects than necessary now, but those does seem safest given that we have those weird, merged FB contacts floating around.

Thanks!  r=me

Comment 4

[Francisco Jordano [:arcturus] [:francisco]]

Landed:

https://github.com/mozilla-b2g/gaia/commit/e3323f920a0a40f5efb386e4a3e48ba8c7de3a81

Comment 5

[Phil Ringnalda (:philor)]

Test bustage (https://tbpl.mozilla.org/php/getParsedLog.php?id=29460457&tree=B2g-Inbound), could you please revert that?

Comment 6

[Jason Smith [:jsmith]]

(In reply to Phil Ringnalda (:philor) from comment #5)
> Test bustage
> (https://tbpl.mozilla.org/php/getParsedLog.php?id=29460457&tree=B2g-Inbound),
> could you please revert that?

That's strange - the patch here is supposed to fix that regression.

Ben?

Comment 7

[Jason Smith [:jsmith]]

oh and another thing - reverting the patch here won't do any help. The test will likely fail without the patch anyways. We probably should disable the test and figure out why it's failing.

Comment 8

[Phil Ringnalda (:philor)]

Oh, I probably got thrown off by the way b2g-inbound builds whichever pushes it feels like, and then runs tests on whichever of those it feels like, when it gets busy. Odds are what I meant was that bug 929078 jumped the gun and reenabled a failing test.

Comment 9

[Phil Ringnalda (:philor)]

Mmm, nope. https://tbpl.mozilla.org/?showall=1&tree=B2g-Inbound&rev=dd3604f309fd&jobname=gaia-ui-test which reenabled tests was green, https://tbpl.mozilla.org/?showall=1&tree=B2g-Inbound&rev=2f337b145031&jobname=gaia-ui-test which landed after it was green, https://tbpl.mozilla.org/?showall=1&tree=B2g-Inbound&rev=ad69ff83ed54&jobname=gaia-ui-test which was this bug was quite red.

Comment 10

[Jason Smith [:jsmith]]

Okay. Let's get this backed out then.

Comment 11

[Jason Smith [:jsmith]]

Talked with lightsofapollo in IRC - We think we should xfail the test and let bkelly investigate this. We're puzzled by the results in comment 9.

Comment 12

[James Lal [:lightsofapollo]]

I disabled the test here: https://github.com/mozilla-b2g/gaia/commit/6b4f0746b30a8685600435ea96808fc4e1671473

This is only for TBPL- I am suspicious of the failures here being a real functional problem rather then just a test error.

Comment 13

[Jason Smith [:jsmith]]

needinfo on Ben to investigate the test being disabled

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

It appears you cannot initialize a mozContact with an existing mozContact.  Patches being investigated in bug 929287.