gfxQPainterSurface.h should only be included if we are enabling qt builds

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: anarchy, Assigned: anarchy)

Tracking

Trunk
mozilla2.0b7
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

../../staticlib/libthebes.a(gfxASurface.o): In function
`gfxASurface::Wrap(_cairo_surface*)':
gfxASurface.cpp:(.text+0x1d1): undefined reference to
`gfxQPainterSurface::gfxQPainterSurface(_cairo_surface*)'
collect2: ld returned 1 exit status
Attachment #475059 - Attachment is patch: true
Attachment #475059 - Attachment mime type: application/octet-stream → text/plain
Attachment #475059 - Flags: review?(roc)
Sorry for the double work here roc, but I screwed up the patch. It needed to be #ifdef defined(CAIRO_HAS_QT_SURFACE) && defined(MOZ_WIDGET_QT) 

#ifdef doesn't support complex expressions and doesn't evaluate the defined(MOZ_WIDGET_QT) check.
Attachment #475059 - Attachment is obsolete: true
Attachment #475264 - Flags: review?(roc)
review carried over small change from ifdef to if as we are checking two defines.
Attachment #475264 - Attachment is obsolete: true
Attachment #475742 - Flags: review+
Keywords: checkin-needed
Once again I missed a single change of ifdef to if for the defines to be used correctly.
Attachment #475742 - Attachment is obsolete: true
Attachment #476444 - Flags: review+
(In reply to comment #3)
> Created attachment 476444 [details] [diff] [review]
> check_for_moz_widget_qt_before_including_gfxQPainterSurface-v0.3
> 
> Once again I missed a single change of ifdef to if for the defines to be used
> correctly.

I should have noted without this change we would have broken qt builds.
I guess you should ask approval before checkin-needed
Keywords: checkin-needed
Attachment #476444 - Flags: approval2.0?
I think this is safe to push
Roc, can you approve this patch? (considering you reviewed it)
Assignee: nobody → anarchy
Status: NEW → ASSIGNED
Version: unspecified → Trunk
The more I look at this, the more I do not believe this is best fix, it seems we should fix configure.in, so it respects the default-toolkit which it obviously is not doing. Based of what I see in configure.in --default-toolkit=cairo-qt needs to be passed in order to enable MOZ_WIDGET_TOOLKIT=qt which then runs 

     if test "$MOZ_WIDGET_TOOLKIT" = "qt"; then
         QT_SURFACE_FEATURE="#define CAIRO_HAS_QT_SURFACE 1"
     fi

which should only be possible like I said before if default-toolkit=qt was passed to configure.
Can you attach new  patch?
Pushed as requested by Jory:
http://hg.mozilla.org/mozilla-central/rev/f8854fb6b63f

Open another bug if there is a better way to fix it.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
For some reasons the attached patch didn't had the fixes from comment 2 and comment 3.

Re-pushed:
http://hg.mozilla.org/mozilla-central/rev/dd06023bddcd
You need to log in before you can comment on or make changes to this bug.