Comment 30 for bug 2011751

Revision history for this message
Dan Bungert (dbungert) wrote :

Thanks for the patch, Aaron. I'm able to reproduce this bug using the
firefox f11 -> open link in new window, when using the live session of
the daily-live lubuntu iso. And the attached patch does seem to help.

Overall the patch is what I was hoping to see, but I will request some
changes.

* The Maintainer field currently reads "Maintainer: Lubuntu
  Developers". I'd like you to go with the more conventional "Ubuntu
  Developers" attribution. `seeded-in-ubuntu` says that openbox is
  also used in Ubuntu Mate, and of course people can use openbox
  otherwise. The `update-maintainer` script is a convenient way to set
  this field to the value I'm suggesting (when starting from the
  original value).
* There is a sponsorship process listed on the wiki, I think
  https://wiki.ubuntu.com/MOTU/Sponsorship/SponsorsQueue is a good
  summary. This can also help, in the future, find someone to take a
  look at the patch as doing so means your work show up in the queue.
  http://reports.qa.ubuntu.com/reports/sponsoring/
  Since I'm already looking at this I won't demand the SponsorsQueue
  bug changes but please keep that in mind for the next one.
* I forget where I saw it but I like the convention that the patch has
  a change number, like it looks like you're on v4 of the patch. So
  it might have been called glib_crash_bugfix-v4.patch or something.
  A nice convention for when these keep changing.
* One item also mentioned on the wiki is forwarding this patch to
  Debian. It is likely that the same problem applies there.
  https://wiki.ubuntu.com/Debian/Bugs talks about this in detail.
  I will ask that you do this before upload. For this particular bug
  I'd feel comfortable sharing the patch with Debian without
  reproducing on Debian, since it's already known to be happening
  elsewhere. Just say so when forwarding.
* A tweak to the changelog would be nice. It isn't necessary to note
  the update to the maintainer field, that will be the case for most
  if not all packages with an Ubuntu delta. Also, the first segment
  about the patch is in my opinion too developer focused. I'd
  probably start with "Cherry-pick patch from A for crash issue when
  B + C happens", fill in appropriate details you think might be
  interesting for someone who might not look at the code.

So Maintainer + Debian + Changelog tweaks and I will be happy to
upload.