Closed Bug 1434018 Opened 4 years ago Closed 4 years ago
Remove references to non-existent overlay
There are a few references in layout-debug/ to tasksOverlay.xul which no longer exists.
I would like to work on this bug. Can you assign this to me?
I have uploaded the patch. I have made all the changes
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 on attachment 8946605 [details] [diff] [review] bug-1434018.patch Forwarding the review request along...
Attachment #8946605 - Flags: review?(dbaron)
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 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-.
Attachment #8946605 - Flags: review?(dbaron) → review-
... and also comment 5.
I have made all the mentioned changes.
Attachment #8948198 - Flags: review?(dbaron)
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.
Attachment #8948198 - Flags: review?(dbaron) → review+
Attachment #8948434 - Flags: review?(dbaron)
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/aad65073f756 Remove references to old (SeaMonkey) overlays from layoutdebug code. r=dbaron
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.
You need to log in before you can comment on or make changes to this bug.