386917 - "Choose Application" from feed drop down menu doesn't work anymore

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

When clicking on the "Choose Application" from the feed drop down menu, you should get the filepicker dialog. This stopped working as soon as the patch for bug 279703 got checked in.

Comment 1

[Neil Deakin]

This is because the command event now fires after the open attribute is already removed. This 'feature' needed for this is tricky to implement without it being hacky because the command needs to close the menu then open a modal dialog.

Comment 2

[pal-moz]

select "Choose Application" and click "Subscribe Now", then you can choose application.

Comment 3

[Neil Deakin]

Attachment: add a state api to popups

This allows the choose application dialog handling to check if the popup is being hidden. Martijn, does this patch fix what it needed here?

Comment 4

[Martijn Wargers (dead)]

Yeah, that patch fixes it for me.

Comment 5

[Neil Deakin]

Attachment: add some code to the popup tests to check the state

Comment 6

[Asaf Romano (gone)]

Comment on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

in feedwriter you should probably get the state manually, from its boxObject. wrappedJSObject usage in this component is a can of worms (well, used to be).

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

With what mano said, sr=bzbarsky

Comment 8

[Neil Deakin]

(In reply to comment #6)
> (From update of attachment 275974 [details] [diff] [review])
> in feedwriter you should probably get the state manually, from its boxObject.
> wrappedJSObject usage in this component is a can of worms (well, used to be).
>

OK I'll make that change, although in the future I'd rather reduce the number of calls to box object methods.

Comment 9

[Asaf Romano (gone)]

Comment on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

With that, r=mano.

Comment 10

[Neil Deakin]

Attachment: updated patch for checkin

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

a1.9=dbaron