879917 - the WebIDL parser should more gracefully handle dictionaries being defined twice.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Trevor Saunders (:tbsaunde)]

This can happen if you say accidentally add the webidl file to the list of files twice.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So the issue is that IDLScope.resolveIdentifierConflict does this:

268         if originalObject.tag == IDLInterfaceMember.Tags.Method and \
269            newObject.tag == IDLInterfaceMember.Tags.Method:
270             return originalObject.addOverload(newObject)

But if originalObject is an IDLDictionary, say, it has no .tag.

Kyle, can/should we just use isinstance here instead?

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

That seems reasonable to me.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Need to land the fix for bug 946897 and then see what's left to do here...

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Is there anything left to do after bug 946897 was landed? If yes, I would like to take this as my first bug. Please tell me how to reproduce this issue. Thanks.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Sorry for the lag; it was the weekend...

Yes, there's something to do here.  You can reproduce by adding these two lines to any WebIDL file:

  dictionary Foo {};
  dictionary Foo {};

and then seeing the totally incomprehensible exception that gets thrown when you build in dom/bindings.

If you want to work on this, that sounds great!  I'm happy to answer any questions you have; response times should be smaller during the week.  ;)

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: part 1: Strip trailing whitespaces in WebIDL.py

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: part 2: Emit useful message on dictionary name collisions

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8417250 [details] [diff] [review]
part 1: Strip trailing whitespaces in WebIDL.py

r=me

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8417251 [details] [diff] [review]
part 2: Emit useful message on dictionary name collisions

Shouldn't that be "or" instead of "and"?

r=me with that change or an explanation of why it's "and".

Comment 11

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: part 1: Strip trailing whitespaces in WebIDL.py.

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8417799 [details] [diff] [review]
part 1: Strip trailing whitespaces in WebIDL.py.

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

Carry over r+.

Comment 13

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: part 2: Emit useful message on dictionary name collisions.

Comment 14

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8417799 [details] [diff] [review]
part 1: Strip trailing whitespaces in WebIDL.py.

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

Carry over r+.

Comment 15

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

(In reply to Ting-Yu Lin [:TYLin] from comment #13)
> Created attachment 8417803 [details] [diff] [review]
> part 2: Emit useful message on dictionary name collisions.

It should be "or". Using "and" generates exception on the following test case:

callback Foo = void (int foo);
dictionary Foo {};

bz, thanks for your review!

Comment 16

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: part 2: Emit useful message on dictionary name collisions.

Comment 17

[Carsten Book [:Tomcat]]

Hi, could you provide a Try link. Suggestions for what to run if you haven't yet can be found here:
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 18

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Since I modified only the WebIDL parser, it should be sufficient to test the build status.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=fd05455cd50a

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9e5847428d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/aceefcfd3708

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c9e5847428d5
https://hg.mozilla.org/mozilla-central/rev/aceefcfd3708