360119 - identify and fix the XUL overlays using processing instruction inside the <overlay> element

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Nickolay_Ponomarev]

Per bug 359959 comment 16, these should be fixed. It's possible that this won't be supported in the new patch in bug 319654.

These are the two examples I've seen:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/base/content/viewSourceOverlay.xul&rev=1.6&mark=42
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/base/content/macBrowserOverlay.xul&rev=1.10&mark=51-56

Comment 1

[Boris Zbarsky [:bzbarsky]]

What about seamonkey, thunderbird, etc?  Do they not have any files with this issue?

Comment 2

[Nickolay_Ponomarev]

They probably do, but we don't have a component for "fix something in the whole tree", so I filed the bug here. I'll probably write something to check this automatically.

Comment 3

[Neil Deakin]

If this is something that authors have used often, and will now not work the same way, then you should also post a message to m.d.platform and the project_owners list.

Comment 4

[Nickolay_Ponomarev]

Neil, sure.

A quick script (looking for "<overlay" or ":overlay" before "<?") weirdly only finds hits in browser/ (of the whole tree). Some of these may be false positives, although that's not very likely:

c:\mozilla\browser\base\content\downloadManagerOverlay.xul: FAIL
c:\mozilla\browser\base\content\extensionsManagerOverlay.xul: FAIL
c:\mozilla\browser\base\content\jsConsoleOverlay.xul: FAIL
c:\mozilla\browser\base\content\macBrowserOverlay.xul: FAIL
c:\mozilla\browser\base\content\softwareUpdateOverlay.xul: FAIL
c:\mozilla\browser\base\content\viewSourceOverlay.xul: FAIL

Comment 5

[Nickolay_Ponomarev]

Attachment: patch

Unfortunately I can't test this, since I don't have a Mac.

Comment 6

[Asaf Romano (gone)]

Comment on attachment 245330 [details] [diff] [review]
patch

r=mano.

Comment 7

[Boris Zbarsky [:bzbarsky]]

What about browser/base/content/macBrowserOverlay.xul ?

Comment 8

[Phil Ringnalda (:philor)]

Attachment: With macBrowserOverlay.xul

Indeed, it works much better including the one that crashes us at startup :)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Probably need to move the xul-stylesheet PIs there too....

Comment 10

[Phil Ringnalda (:philor)]

Comment on attachment 245343 [details] [diff] [review]
With macBrowserOverlay.xul

Oops, needed to move the stylesheets, too.

Comment 11

[Phil Ringnalda (:philor)]

Attachment: With macBrowserOverlay.xul v.2

With *all* of it, this time, I hope.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Doesn't that last patch change the order of the overlay and stylesheets?  I doubt that's really needed (or desirable).

Comment 13

[Phil Ringnalda (:philor)]

Attachment: With macBrowserOverlay.xul v.3

Sigh. I was also putting them between the (#included) doctype and the <overlay> element, which whether or not it matters just isn't done.

Comment 14

[Asaf Romano (gone)]

Comment on attachment 245348 [details] [diff] [review]
With macBrowserOverlay.xul v.3

r=mano

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 16

[Nickolay_Ponomarev]

Phil, thanks for catching that!

Comment 17

[Nickolay_Ponomarev]

Edited http://developer.mozilla.org/en/docs/Firefox_3_for_developers to mention this and posted this: http://groups-beta.google.com/group/mozilla.dev.tech.xul/browse_frm/thread/1d4be633082f5099/36085ddb5b956a58#36085ddb5b956a58

Comment 18

[amiran]

??

Comment 19

[amiran]

what i can use this BUG

Comment 20

[Alex Vincent [:WeirdAl]]

How can a bug that's already fixed block the release?

Comment 21

[Dave Townsend [:mossop]]

We could probably do with updating the examples on MDC to stop recommending this

Comment 22

[Nickolay_Ponomarev]

Are there any specific examples in need of update?

Comment 23

[Dave Townsend [:mossop]]

Of course I can't find the one I copied from earlier now, but there is definitely some example there.

Comment 24

[Eric Shepherd [:sheppy]]

Found one case where this was done and changed it.  Marking as doc-complete; if anyone finds other examples of this, fix 'em. :)