Last Comment Bug 528941 - pangox.h includes should be removed
: pangox.h includes should be removed
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 1.9.2 Branch
: x86 Linux
: -- trivial (vote)
: mozilla1.9.3a1
Assigned To: Nirbheek Chauhan
:
Mentors:
http://bugs.gentoo.org/show_bug.cgi?i...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-16 07:40 PST by Nirbheek Chauhan
Modified: 2009-11-18 11:30 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta4-fixed


Attachments
Remove pangox.h includes (802 bytes, patch)
2009-11-16 11:48 PST, Nirbheek Chauhan
karlt: review+
dbaron: approval1.9.2+
Details | Diff | Splinter Review

Description Nirbheek Chauhan 2009-11-16 07:40:43 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b2) Gecko/20091112 Gentoo Firefox/3.6b2
Build Identifier: 

Sometime during the 1.9.1 cycle, pangox got replaced by pangoxft for fonts, but the headers are still around. pangox has been deprecated[1][2] upstream for a while now, hence there's no point keeping the header includes around.

This is part of our downstream (Gentoo) effort[3] to remove usage of pangox in ebuilds.

# pango/pangox.h is included
$ grep -re pangox\.h .
./config/system-headers:pango/pangox.h
./js/src/config/system-headers:pango/pangox.h
./gfx/src/thebes/nsThebesDeviceContext.cpp:#include <pango/pangox.h>

# But no pango_x_* functions are used
$ grep -re pango_x_ .
$

This seems to render bug 323671 obsolete as well.

1. http://git.gnome.org/cgit/pango/tree/pango/pangox.h#n33
2. http://library.gnome.org/devel/pango/stable/pango-X-Fonts-and-Rendering.html
3. http://bugs.gentoo.org/show_bug.cgi?id=293368

Reproducible: Always




I have tested that removing pangox.h from includes (and from the system) allows xulrunner to build without any problems, and firefox starts up without any problems as well.
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-16 08:55:36 PST
Were you going to post the patch?
Comment 2 Nirbheek Chauhan 2009-11-16 11:48:30 PST
Created attachment 412656 [details] [diff] [review]
Remove pangox.h includes

Right, the patch is pretty straightforward :)
Comment 3 Karl Tomlinson (:karlt) 2009-11-16 13:27:06 PST
Comment on attachment 412656 [details] [diff] [review]
Remove pangox.h includes

Thank you.  Please add the checkin-needed keyword if you'd like someone to push this to trunk.

This would be fine on 1.9.2 also, but that's not my decision to make.
Comment 4 Nirbheek Chauhan 2009-11-16 13:42:00 PST
(In reply to comment #3)
> (From update of attachment 412656 [details] [diff] [review])
> Thank you.  Please add the checkin-needed keyword if you'd like someone to push
> this to trunk.
> 

Added, thank you for your help

> This would be fine on 1.9.2 also, but that's not my decision to make.

I added "wanted1.9.2 ?" since this also affects the ebuilds we have which use the official release packages (instead of building from source). Hence we'd like it if this could go into 1.9.2.
Comment 5 Dão Gottwald [:dao] 2009-11-18 05:22:15 PST
http://hg.mozilla.org/mozilla-central/rev/dca9a50280cd
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-18 08:23:11 PST
Comment on attachment 412656 [details] [diff] [review]
Remove pangox.h includes

a1.9.2=dbaron

Note You need to log in before you can comment on or make changes to this bug.