Closed
Bug 1278130
Opened 8 years ago
Closed 8 years ago
Port Bug 1273685 PopupBlocking:UpdateBlockedPopups messages can be very large to Seamonkey
Categories
(SeaMonkey :: Passwords & Permissions, defect)
SeaMonkey
Passwords & Permissions
Tracking
(seamonkey2.46 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)
RESOLVED
FIXED
People
(Reporter: frg, Assigned: frg)
References
Details
Attachments
(1 file, 2 obsolete files)
4.35 KB,
patch
|
frg
:
review+
philip.chee
:
approval-comm-aurora+
philip.chee
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
Showing individual popup windows is currently broken in the status and notification bar. Instead you see an error in the web console. >> Timestamp: 6/5/2016 10:15:31 PM >> Error: TypeError: popups[i] is undefined >> Source File: chrome://communicator/content/utilityOverlay.js >> Line: 1287 Bug 1273685 needs to be ported.
Assignee | ||
Comment 1•8 years ago
|
||
A patch! A patch!
Assignee | ||
Updated•8 years ago
|
status-seamonkey2.46:
--- → affected
status-seamonkey2.47:
--- → affected
Assignee | ||
Comment 2•8 years ago
|
||
oops...
Attachment #8763335 -
Attachment is obsolete: true
Attachment #8763335 -
Flags: review?(philip.chee)
Attachment #8763336 -
Flags: review?(philip.chee)
Assignee | ||
Updated•8 years ago
|
status-seamonkey2.48:
--- → affected
Comment 3•8 years ago
|
||
Comment on attachment 8763336 [details] [diff] [review] 1278130-popupblocking-V1a.patch > + (oldBrowser.blockedPopups && !newBrowser.blockedPopups) || > + (!oldBrowser.blockedPopups && newBrowser.blockedPopups)) This looks like it's attempting to execute a logical XOR. If so you could do the following: (!oldBrowser.blockedPopups != !newBrowser.blockedPopups) > + // Check for duplicates and reuse the old menuitem. I think this is wrong. Every time you call retrieveListOfBlockedPopups() you get a potentially different list of popups. We should call RemovePopupsItems() every time we call onpopuphiding. Should we port this bit too? https://hg.mozilla.org/mozilla-central/annotate/01748a2b1a463f24efd9cd8abad9ccfd76b037b8/browser/base/content/browser.js#l557 Otherwise this looks reasonable.
Attachment #8763336 -
Flags: review?(philip.chee) → review+
Assignee | ||
Comment 4•8 years ago
|
||
[Approval Request Comment] Regression caused by (bug #): 1273685 User impact if declined: individual popup blocking does not work Testing completed (on m-c, etc.): c-a c-b Risk to taking this patch (and alternatives if risky): non already broken String changes made by this patch: none. xor comment addressed. review+ from Philip Chee carried forward. >> I think this is wrong. Every time you call retrieveListOfBlockedPopups() you >> get a potentially different list of popups. We should call >> RemovePopupsItems() every time we call onpopuphiding. Looking at the code I think RemovePopupsItems() is called already. This is imho only for cases where there are duplicate items in the list itself. I updated the comment for it. Lmk if you think I am wrong here. >> Should we port this bit too? Yupp but I would opt for a separate bug.
Attachment #8763336 -
Attachment is obsolete: true
Attachment #8785597 -
Flags: review+
Attachment #8785597 -
Flags: approval-comm-beta?
Attachment #8785597 -
Flags: approval-comm-aurora?
Assignee | ||
Comment 5•8 years ago
|
||
>> >> Should we port this bit too? >> Yupp but I would opt for a separate bug. I now looked up what the code actually does. Does no longer apply after I reworked the menu in Bug 1055954. The status bar and preferences bar menus do not use a seperator.
Comment 6•8 years ago
|
||
Comment on attachment 8785597 [details] [diff] [review] 1278130-popupblocking-V2.patch (In reply to Frank-Rainer Grahl from comment #5) > >> >> Should we port this bit too? > > >> Yupp but I would opt for a separate bug. > > I now looked up what the code actually does. Does no longer apply after I > reworked the menu in Bug 1055954. The status bar and preferences bar menus > do not use a seperator. Sounds good. a=me for landing on comm-aurora and comm-beta
Attachment #8785597 -
Flags: approval-comm-beta?
Attachment #8785597 -
Flags: approval-comm-beta+
Attachment #8785597 -
Flags: approval-comm-aurora?
Attachment #8785597 -
Flags: approval-comm-aurora+
Assignee | ||
Comment 7•8 years ago
|
||
Locked and bolted: https://hg.mozilla.org/comm-central/rev/ae490e13b85f https://hg.mozilla.org/releases/comm-aurora/rev/30533da241d1 https://hg.mozilla.org/releases/comm-beta/rev/6ba8abbc0969
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•