Closed Bug 347144 Opened 18 years ago Closed 18 years ago

Bookmark context menus have never worked properly

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

Details

(Keywords: fixed-seamonkey1.1a, fixed1.8.1)

Attachments

(1 file, 1 obsolete file)

Comment on attachment 101613 [details] [diff] [review]
Patch to support context menus on menupopups

>Index: base/src/nsPopupSetFrame.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/layout/xul/base/src/nsPopupSetFrame.cpp,v
>retrieving revision 1.92
>diff -u -r1.92 nsPopupSetFrame.cpp
>--- base/src/nsPopupSetFrame.cpp	19 Aug 2002 18:29:28 -0000	1.92
>+++ base/src/nsPopupSetFrame.cpp	3 Oct 2002 23:48:14 -0000
>@@ -423,6 +442,24 @@
>     OpenPopup(entry, PR_FALSE);
>     entry->mPopupType.SetLength(0);
>   
>+    if (aDestroyEntireChain && entry->mElementContent && entry->mPopupType == NS_LITERAL_STRING("context")) {
entry->mPopupType == NS_LITERAL_STRING("context") is hardly likely to be true here after you've set its length to 0 ...
Attached patch Proposed patch (obsolete) — Splinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #231928 - Flags: superreview?(bzbarsky)
Attachment #231928 - Flags: review?(bzbarsky)
Summary: Bookmark context have never worked properly → Bookmark context menus have never worked properly
Comment on attachment 231928 [details] [diff] [review]
Proposed patch

Seems reasonable, but I don't know this code all that well... make sure to test thoroughly!
Attachment #231928 - Flags: superreview?(bzbarsky)
Attachment #231928 - Flags: superreview+
Attachment #231928 - Flags: review?(bzbarsky)
Attachment #231928 - Flags: review+
(In reply to comment #2)
>(From update of attachment 231928 [details] [diff] [review] [edit])
>Seems reasonable, but I don't know this code all that well... make sure to test
>thoroughly!
Well, I only tested the action that now uses this conditional code path to ensure that it now closed the bookmarks menus correctly when it didn't before.

If you're worried about the string being truncated, I can do that after that conditional block (there's a bunch of other fields being reset anyway).
Yeah, might be good to continue truncating unless we know for sure we don't need to....
Attachment #231928 - Attachment is obsolete: true
Fix checked in to the trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 232147 [details] [diff] [review]
Updated for check in
[Checkin: Comment 6 & 9]

We'd like a feature in SeaMonkey 1.1 that depends on this fix.
Attachment #232147 - Flags: approval1.8.1?
Comment on attachment 232147 [details] [diff] [review]
Updated for check in
[Checkin: Comment 6 & 9]

a=schrep for drivers.
Attachment #232147 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 232147 [details] [diff] [review]
Updated for check in
[Checkin: Comment 6 & 9]


Checkin: {
2006-08-06 12:52	neil%parkwaycc.co.uk 	mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.123.10.6 	MOZILLA_1_8_BRANCH
}
Attachment #232147 - Attachment description: Updated for check in → Updated for check in [Checkin: Comment 6 & 9]
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.

Attachment

General

Created:
Updated:
Size: