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

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
XUL
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Neil Deakin)

Tracking

({regression})

Trunk
mozilla1.9alpha8
x86
Windows XP
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
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.
Assignee: nobody → enndeakin
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9beta1

Comment 2

11 years ago
select "Choose Application" and click "Subscribe Now", then you can choose application.
(Assignee)

Comment 3

10 years ago
Created attachment 275654 [details] [diff] [review]
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?
(Reporter)

Comment 4

10 years ago
Yeah, that patch fixes it for me.
(Assignee)

Comment 5

10 years ago
Created attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state
Attachment #275654 - Attachment is obsolete: true
Attachment #275974 - Flags: superreview?(bzbarsky)
Attachment #275974 - Flags: review?(mano)
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 on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

With what mano said, sr=bzbarsky
Attachment #275974 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 8

10 years ago
(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 on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

With that, r=mano.
Attachment #275974 - Flags: review?(mano) → review+
(Assignee)

Comment 10

10 years ago
Created attachment 276772 [details] [diff] [review]
updated patch for checkin
Attachment #276772 - Flags: approval1.9?
(Assignee)

Updated

10 years ago
Attachment #276772 - Flags: approval1.9?
(Assignee)

Updated

10 years ago
Attachment #275974 - Flags: approval1.9?
Comment on attachment 275974 [details] [diff] [review]
add some code to the popup tests to check the state

a1.9=dbaron
Attachment #275974 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

Updated

9 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.