Closed Bug 1008585 Opened 6 years ago Closed 6 years ago

Fix popup blocker fallout from bug 933462

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set

Tracking

(seamonkey2.27 unaffected, seamonkey2.28 fixed, seamonkey2.29 wontfix, seamonkey2.30 fixed)

RESOLVED FIXED
seamonkey2.29
Tracking Status
seamonkey2.27 --- unaffected
seamonkey2.28 --- fixed
seamonkey2.29 --- wontfix
seamonkey2.30 --- fixed

People

(Reporter: neil, Assigned: neil)

Details

Attachments

(1 file, 1 obsolete file)

Bug 933462 mysteriously changed the type of popupWindowURI from URI to string. We were only using the spec anyway, so this is no great loss. It also regenerates the list of blocked popups each time, so we have to filter them when we build the menu instead.
Attached patch Proposed patch (obsolete) — Splinter Review
Comment on attachment 8420544 [details] [diff] [review]
Proposed patch

Bah, silly file picker taking its time to close, making me hit Enter too many times :-(
Attachment #8420544 - Attachment description: 1008585.diff → Proposed patch
Attachment #8420544 - Flags: review?(philip.chee)
Attached patch Fixed patchSplinter Review
Previous patch only made the menu appear to display correctly the first time. This now fixes using the menu on more than one page, and also correctly unblocks the popup. I also didn't like the duplicate filtering code so I rewrote it.
Assignee: nobody → neil
Attachment #8420544 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8420544 - Flags: review?(philip.chee)
Attachment #8420578 - Flags: review?(philip.chee)
Comment on attachment 8420578 [details] [diff] [review]
Fixed patch

r=me
Attachment #8420578 - Flags: review?(philip.chee) → review+
Pushed comm-central changeset abfea2c23414.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.29
Comment on attachment 8420578 [details] [diff] [review]
Fixed patch

[Approval Request Comment]
Regression caused by (bug #): 933462
User impact if declined: Popup blocker menu does not work
Testing completed (on m-c, etc.): Landed on c-c
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None
Attachment #8420578 - Flags: approval-comm-aurora?
Comment on attachment 8420578 [details] [diff] [review]
Fixed patch

a=me commm-aurora
Attachment #8420578 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 8420578 [details] [diff] [review]
Fixed patch

[Approval Request Comment]
Regression caused by (bug #): 1024391 backed out 933462
User impact if declined: Can't unblock popups
Testing completed (on m-c, etc.): N/A, backout
Risk to taking this patch (and alternatives if risky): None, backout
String changes made by this patch: None
Attachment #8420578 - Flags: approval-comm-beta?
Attachment #8420578 - Flags: approval-comm-beta? → approval-comm-beta+
I managed to confuse myself over the tracking flags, sorry.

As per 933462 I've set the tracking flag for 2.30 to fixed to show that although the patch was backed out of 2.29 this was after it was uplifted. (Unlike 933462 we don't have a "disabled" state, so I've chosen "wontfix" as the nearest equivalent.)
You need to log in before you can comment on or make changes to this bug.