1434018 - Remove references to non-existent overlay

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Brendan Dahl [:bdahl]]

There are a few references in layout-debug/ to tasksOverlay.xul which no longer exists.

Comment 1

[abhinavdv]

I would like to work on this bug. Can you assign this to me?

Comment 2

[abhinavdv]

Attachment: bug-1434018.patch

I have uploaded the patch. I have made all the changes

Comment 3

[Brendan Dahl [:bdahl]]

Comment on attachment 8946605 [details] [diff] [review]
bug-1434018.patch

I'm not a peer or owner the layout module. You'll need someone from this list: https://wiki.mozilla.org/Modules/All#Layout_Engine

Comment 4

[Brendan Dahl [:bdahl]]

Comment on attachment 8946605 [details] [diff] [review]
bug-1434018.patch

Forwarding the review request along...

Comment 5

[Brendan Dahl [:bdahl]]

It looks like globalOverlay.xul in layout/tools/layout-debug/ui/content/layoutdebug.xul is not a valid file. Can you remove that too?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8946605 [details] [diff] [review]
bug-1434018.patch

So the existing layout-debug code has overlays into both Firefox and SeaMonkey.  I think this is mostly removing the SeaMonkey overlay, but not completely.  There is, for example, the code lower down in layoutdebug-overlay.xul that should also be removed.

(I'm not sure if anyone's tested in the SeaMonkey overlay works for a decade or so... but there are certainly more references to tasksOverlay.xul in comm-central, although I don't actually see a *file* called tasksOverlay.xul, so I'm guessing it doesn't actually do anything anymore.)

So I think I'm fine with this if you also remove the other bit of the SeaMonkey overlay code in layoutdebug-overlay.xul.  But I'd like to look at the revised patch, so marking as review-.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

... and also comment 5.

Comment 8

[abhinavdv]

Attachment: bug-143018-v2.patch

I have made all the mentioned changes.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8948198 [details] [diff] [review]
bug-143018-v2.patch

So this looks good, except for two things.  First, you should have a commit message that's suitable for committing to our repository.  For some recent examples of commit messages, see:
https://hg.mozilla.org/mozilla-central/pushloghtml .  In particular, you should have:
 * the bug number
 * a brief description of the code change being made
 * the reviewer
so something like:
Bug 1434018 - Remove references to old (SeaMonkey) overlays from layoutdebug code.  r=dbaron

> <?xml-stylesheet href="chrome://communicator/skin/" type="text/css" ?>
> 
>-<?xul-overlay href="chrome://global/content/globalOverlay.xul"?>
>-<?xul-overlay href="chrome://communicator/content/tasksOverlay.xul"?>
>+

Second, there's no need to add a new blank line here.  In fact, it might be worth *removing* one of the blank lines.



So r=dbaron... but could you post a revised patch with those changes, so that somebody can commit it?  And when you do, needinfo? one of us so we can make that happen.

Comment 10

[abhinavdv]

Attachment: bug-143018v3.patch

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aad65073f756
Remove references to old (SeaMonkey) overlays from layoutdebug code.  r=dbaron

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Thanks for the patch!  It's landed now in mozilla-inbound, and should be in mozilla-central (and thus the nightly channel) sometime in the next 24 hours.

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/aad65073f756