The default bug view has changed. See this FAQ.

Move popups, panels, and tooltips into popupsets (e.g. mainPopupset)

RESOLVED FIXED in seamonkey2.1a3

Status

SeaMonkey
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1a3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Firefox Bug 535974 moved remaining stray tooltips and panels into "mainPopupSet". We should do the same.
(Assignee)

Comment 1

7 years ago
Created attachment 451947 [details] [diff] [review]
Patch v1.0

First cut.
Attachment #451947 - Flags: review?(neil)

Comment 2

7 years ago
Comment on attachment 451947 [details] [diff] [review]
Patch v1.0

>+  <popupset id="bookmarksPopupset">
>+    <tooltip  id="btTooltip"/>
>+    <template id="bookmarksMenuTemplate"/>
Templates aren't popups. (Also not sure why btTooltip needs its own popupset.)

>+  </popupset>
> 
> <popup id="msgComposeContext" onpopupshowing="openEditorContextMenu(this);">
Did you deliberately avoid this one?

Comment 3

7 years ago
How much does this clash with my places bookmarks work in bug 498596 which changes some popups on its own?
(Assignee)

Comment 4

7 years ago
> How much does this clash with my places bookmarks work in bug 498596 which
> changes some popups on its own?

Ah I see you move the bookmarks popups into mainPopupSet. I could leave the bookmarks popups alone but you'll still need to rebase or merge.

Comment 5

7 years ago
Shouldn't we be looking to change popup to menupopup? See bug 571567
(Assignee)

Comment 6

7 years ago
Created attachment 454534 [details] [diff] [review]
Patch v1.1 inc menupopups.

> neil@parkwaycc.co.uk      2010-06-17 08:00:33 PDT
> 
> (From update of attachment 451947 [details] [diff] [review])
>>+  <popupset id="bookmarksPopupset">
>>+    <tooltip  id="btTooltip"/>
>>+    <template id="bookmarksMenuTemplate"/>
>Templates aren't popups. (Also not sure why btTooltip needs its own popupset.)
Fixed. and Fixed.

>>+  </popupset>
>> 
>> <popup id="msgComposeContext" onpopupshowing="openEditorContextMenu(this);">
> Did you deliberately avoid this one?
Fixed.

> Robert Kaiser 2010-06-17 09:10:07 PDT
> 
> How much does this clash with my places bookmarks work in bug 498596 which
> changes some popups on its own?

Sorry but it seems no matter what I try I think you still need to rebase.

> Ian Neal      2010-06-17 11:38:33 PDT
> 
> Shouldn't we be looking to change popup to menupopup? See bug 571567
I've converted popup->menupopup for those popups I've found and moved inside popupsets. I may not have caught all occurrences but that can be done in a following patch.
Attachment #451947 - Attachment is obsolete: true
Attachment #454534 - Flags: review?(neil)
Attachment #454534 - Flags: feedback?(iann_bugzilla)
Attachment #451947 - Flags: review?(neil)

Updated

7 years ago
Attachment #454534 - Flags: review?(neil) → review+

Comment 7

7 years ago
(In reply to comment #5)
> Shouldn't we be looking to change popup to menupopup? See bug 571567
stefanh says that Mac checkbox menuitems only work in menupopups, not popups.

Comment 8

7 years ago
Comment on attachment 454534 [details] [diff] [review]
Patch v1.1 inc menupopups.

Not spotted any issues using this patch, may be worth checking with stefanh though.
Attachment #454534 - Flags: feedback?(iann_bugzilla) → feedback+
(Assignee)

Updated

7 years ago
Attachment #454534 - Flags: feedback?(stefanh)

Comment 9

7 years ago
Comment on attachment 454534 [details] [diff] [review]
Patch v1.1 inc menupopups.

Seems slightly bitrottened:
Hunk #3 FAILED at 928.
1 out of 3 hunks FAILED -- saving rejects to file suite/mailnews/mailWindowOverlay.xul.rej

Comment 10

7 years ago
Comment on attachment 454534 [details] [diff] [review]
Patch v1.1 inc menupopups.

But the patch seems to work fine.
Attachment #454534 - Flags: feedback?(stefanh) → feedback+

Updated

7 years ago
Blocks: 578440

Comment 11

7 years ago
(In reply to comment #6)
> I've converted popup->menupopup for those popups I've found and moved inside
> popupsets. I may not have caught all occurrences but that can be done in a
> following patch.

You missed like 10-12 <popup>'s
(Assignee)

Comment 12

7 years ago
Created attachment 457312 [details] [diff] [review]
Patch v1.1a fix bit rot plus utilityOverlay.xul f=IanN f=stefanh r=Neil

Carrying forward r=Neil
Fix bit-rot plus don't know why utilityOverlay.xul wasn't in the previous patch as it was in my local tree.
Attachment #454534 - Attachment is obsolete: true
Attachment #457312 - Flags: review+
Attachment #457312 - Flags: feedback+
(Assignee)

Comment 13

7 years ago
Comment on attachment 457312 [details] [diff] [review]
Patch v1.1a fix bit rot plus utilityOverlay.xul f=IanN f=stefanh r=Neil

> Fix bit-rot plus don't know why utilityOverlay.xul wasn't in the previous patch
> as it was in my local tree.
rs=Neil over IRC
Depends on: 578775
(In reply to comment #13)
> Comment on attachment 457312 [details] [diff] [review]
> Patch v1.1a fix bit rot plus utilityOverlay.xul f=IanN f=stefanh r=Neil
> 
> > Fix bit-rot plus don't know why utilityOverlay.xul wasn't in the previous patch
> > as it was in my local tree.
> rs=Neil over IRC

a+=Callek if you want to land before we reopen
(Assignee)

Updated

7 years ago
Blocks: 578876
(Assignee)

Comment 15

7 years ago
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/04473c202657
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Target Milestone: --- → seamonkey2.1a3
Could this have caused bug 586408? (I just want to direct some knowledgeable people over there to find a fix, not reopen this one.)
You need to log in before you can comment on or make changes to this bug.