578441 - Use <menupopup> instead of <popup>

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Dão Gottwald [:dao]]

Support for <popup> is going away (bug 578322). <popup> has been obsoleted for some time and support was flaky (e.g. bug 571567).

Comment 3

[Nomis101]

Is it as easy at it seems, that you only need to change all "popup" to "menupopup" in all those xul files from the mxr url (like in bug 571567). If yes, than I can make patch.

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> Is it as easy at it seems, that you only need to change all "popup" to
> "menupopup" in all those xul files from the mxr url (like in bug 571567). If
> yes, than I can make patch.

Yes I believe it should be, see also bug 578440 for the suite/ version (you can always just try one or two first and see if it fixes the current popup menu issues).

Comment 5

[Ludovic Hirlimann [:Usul]]

I'm compiling the patch right now.

Comment 6

[Nomis101]

(In reply to comment #5)
> I'm compiling the patch right now.

Oh, I'am ready with my patch. Should I upload it anyway?

Comment 7

[Ludovic Hirlimann [:Usul]]

Attachment: patch fixing the issue

Standrad8 waht the requirements review wise for the parts in suite/ is asking your enough ?

Comment 8

[Nomis101]

Your patch look very similar to mine :-) The only difference is, you didn't realign the onpopup's under the "id", like:

-  <popup id="mailContext"
-         onpopupshowing="return fillMailContextMenu(event);"
-         onpopuphiding="mailContextOnPopupHiding(event);">
+  <menupopup id="mailContext"
+             onpopupshowing="return fillMailContextMenu(event);"
+             onpopuphiding="mailContextOnPopupHiding(event);">

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 458328 [details] [diff] [review]
patch fixing the issue

This looks fine, but please fix the indentation in multiple places, as per comment 8.

Comment 11

[Ludovic Hirlimann [:Usul]]

Attachment: With ident fixed

Comment 12

[Ludovic Hirlimann [:Usul]]

(In reply to comment #10)
> Comment on attachment 458328 [details] [diff] [review]
> patch fixing the issue
> 
> This looks fine, but please fix the indentation in multiple places, as per
> comment 8.

done

Comment 13

[Mark Banner (:standard8)]

You missed a couple, but I fixed them on checkin:

http://hg.mozilla.org/comm-central/rev/5e29e16c191b