The default bug view has changed. See this FAQ.

--enable-system-cairo build is broken after Bug 715658 fixed

RESOLVED FIXED in mozilla24

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: ojab, Assigned: ojab)

Tracking

Trunk
mozilla24
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(10 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
/sources/mozilla-inbound/gfx/thebes/gfxPlatform.cpp: In member function ‘virtual mozilla::RefPtr<mozilla::gfx::SourceSurface> gfxPlatform::GetSourceSurfaceForSurface(mozilla::gfx::DrawTarget*, gfxASurface*)’:
/sources/mozilla-inbound/gfx/thebes/gfxPlatform.cpp:516:53: error: ‘cairo_null_surface_create’ was not declared in this scope
/sources/mozilla-inbound/gfx/thebes/gfxPlatform.cpp:521:95: error: ‘cairo_surface_attach_snapshot’ was not declared in this scope
make[7]: *** [gfxPlatform.o] Error 1
(Assignee)

Updated

5 years ago
Blocks: 715658

Comment 1

5 years ago
Confirmed, I'm also seeing this bug. If I revert the patch attached to bug 715658 then the build succeeds, another option is to remove --enable-system-cairo from my mozconfig; otherwise the build fails with the error above.
When I was reviewing bug 715658 I took the fact that it would break system cairo into account. System-cairo is not really a supported configuration and I decided that a quick, and reliably correct solution was better than trying to avoid exposing the new api to cairo. That being said, our reliance on this API should be temporary and when we don't need it anymore I'd like to get system cairo working again. I expect, I'd also take a patch that removed our dependency on this API. In the meantime we should probably make configure fail when --enable-system-cairo is specified.
Created attachment 594066 [details] [diff] [review]
Fail in configure when --enable-system-cairo is used

This should be the patch suggested by jrmuizel.
Attachment #594066 - Flags: review?(jmuizelaar)
Comment on attachment 594066 [details] [diff] [review]
Fail in configure when --enable-system-cairo is used

Looks fine to me but I don't read autoconf very well. If you're not sure about it's correctness please get a review from a better reviewer.
Attachment #594066 - Flags: review?(jmuizelaar) → review+
Attachment #594066 - Attachment is patch: true
Attachment #594066 - Flags: review?(ted.mielczarek)

Comment 5

5 years ago
(In reply to Bas Schouten (:bas) from comment #3)
> Created attachment 594066 [details] [diff] [review]
> Fail in configure when --enable-system-cairo is used
> 
> This should be the patch suggested by jrmuizel.

Isn't this a bit drastic, when a few simple ifdefs in gfx/thebes/gfxPlatform.cpp
would be sufficient to fix this problem?

Comment 6

5 years ago
Created attachment 595000 [details] [diff] [review]
Ad 2 #ifdefs to gfx/thebes/gfxPlatform.cpp

(In reply to Octoploid from comment #5)
> (In reply to Bas Schouten (:bas) from comment #3)
> Isn't this a bit drastic, when a few simple ifdefs in
> gfx/thebes/gfxPlatform.cpp
> would be sufficient to fix this problem?

Simple? I can do simple! This patch fixes compiling with --enable-system-cairo for me
(In reply to Andrew Benton from comment #6)
> Created attachment 595000 [details] [diff] [review]
> Ad 2 #ifdefs to gfx/thebes/gfxPlatform.cpp
> 
> (In reply to Octoploid from comment #5)
> > (In reply to Bas Schouten (:bas) from comment #3)
> > Isn't this a bit drastic, when a few simple ifdefs in
> > gfx/thebes/gfxPlatform.cpp
> > would be sufficient to fix this problem?
> 
> Simple? I can do simple! This patch fixes compiling with
> --enable-system-cairo for me

It means this bug exists again on linux (once we switch Azure content rendering on there), so it doesn't actually help.

Comment 8

5 years ago
(In reply to Bas Schouten (:bas) from comment #7)
> It means this bug exists again on linux (once we switch Azure content
> rendering on there), so it doesn't actually help.

It means I can compile firefox again which is a step in the right direction. The alternative is to stick with Firefox-10 for the foreseeable future.
(In reply to Andrew Benton from comment #8)
> (In reply to Bas Schouten (:bas) from comment #7)
> > It means this bug exists again on linux (once we switch Azure content
> > rendering on there), so it doesn't actually help.
> 
> It means I can compile firefox again which is a step in the right direction.
> The alternative is to stick with Firefox-10 for the foreseeable future.

Why can't you compile firefox without system cairo?

Comment 10

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> Why can't you compile firefox without system cairo?

Pure stubborn bloodymindedness ;)
I prefer to link to system libraries wherever possible
(In reply to Andrew Benton from comment #10)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> > Why can't you compile firefox without system cairo?
> 
> Pure stubborn bloodymindedness ;)
> I prefer to link to system libraries wherever possible

If that's the reason, you can always apply this patch yourself :)

Comment 12

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> (In reply to Andrew Benton from comment #8)
> > (In reply to Bas Schouten (:bas) from comment #7)
> > > It means this bug exists again on linux (once we switch Azure content
> > > rendering on there), so it doesn't actually help.
> > 
> > It means I can compile firefox again which is a step in the right direction.
> > The alternative is to stick with Firefox-10 for the foreseeable future.
> 
> Why can't you compile firefox without system cairo?

There was a time when Firefox, compiled without system cairo, looked 
**** (color fringes), because the in-tree cairo only included the awful 
LCD_FILTER_LEGACY algorithm. 
I've been using --enable-system-cairo since that time.
But today I've build Firefox without --enable-system-cairo and it actually looks OK now.
So --enable-system-cairo isn't necessary anymore.

Comment 13

5 years ago
Created attachment 595138 [details] [diff] [review]
patch the new functions into cairo-1.10.2

Well if the mountain will not come to Mohammed, perhaps we can patch cairo to have the new functions? This patch adds the new functions into cairo-1.10.2. With the new functions in the system cairo I can compile firefox without patching the firefox code
Comment on attachment 594066 [details] [diff] [review]
Fail in configure when --enable-system-cairo is used

Review of attachment 594066 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +8066,5 @@
>  
>  MOZ_TREE_CAIRO=1
>  MOZ_ARG_ENABLE_BOOL(system-cairo,
>  [  --enable-system-cairo   Use system cairo (located with pkgconfig)],
> +AC_MSG_ERROR([ --enable-system-cairo is no longer supported.]),

Please file a bug on making system cairo work again, and list that bug number in a comment here (and possibly in the error message).
Attachment #594066 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → bas.schouten

Comment 15

5 years ago
doesn't mozilla like to push changes into upstream? 
this way the fix is for real rather than trying to apply workarounds in packages from distros or switching to internal cairo.
(In reply to Ionut Biru from comment #15)
> doesn't mozilla like to push changes into upstream? 
> this way the fix is for real rather than trying to apply workarounds in
> packages from distros or switching to internal cairo.

I've been hesitant to push this upstream because I'm not really sure it's appropriate.

Comment 17

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #16)
> (In reply to Ionut Biru from comment #15)
> > doesn't mozilla like to push changes into upstream? 
> > this way the fix is for real rather than trying to apply workarounds in
> > packages from distros or switching to internal cairo.
> 
> I've been hesitant to push this upstream because I'm not really sure it's
> appropriate.

only one way to find out. let upstream review the changes and try harder to push it there if the review is not favorable. from that, mozilla has more benefits along with it,other communities as well.

Comment 18

5 years ago
Created attachment 609898 [details] [diff] [review]
patch the new functions into cairo-1.12.0

Updated the system cairo patch for cairo-1.12.0

Comment 19

5 years ago
(In reply to Andrew Benton from comment #18)
> Created attachment 609898 [details] [diff] [review]
> patch the new functions into cairo-1.12.0
> 
> Updated the system cairo patch for cairo-1.12.0

Ugh, that patch doesn't work, don't use it. Too much has changed between cairo-1.10.2 and 1.12.0. I don't understand enough to make it work.

Updated

5 years ago
Attachment #595138 - Attachment is obsolete: true

Updated

5 years ago
Attachment #609898 - Attachment is obsolete: true

Comment 20

5 years ago
Created attachment 611180 [details] [diff] [review]
patch the new functions into cairo-1.12.0

Improved patch for cairo-1.12.0. This patch works in that I can compile Firefox with --enable-system-cairo. cairo-1.12.0 is pretty buggy though https://bugs.freedesktop.org/show_bug.cgi?id=47266
https://bugs.gentoo.org/show_bug.cgi?id=409593

Updated

5 years ago
Attachment #595138 - Attachment is obsolete: false

Comment 21

5 years ago
@comment 20: for the moment, it seems most of the problem (in regard of those bugs) lies in xorg-server, not cairo.
Oh noes, not the "let's ditch --enable-system-cairo" argument again... i'll use https://bugzilla.mozilla.org/attachment.cgi?id=595000 in the meantime if it works, but it'd be good to have this properly fixed, as lots of distributors are using that switch. Fx 12.0b5 is broken for me as-is. Jeff, you're saying "our reliance on this API should be temporary", what's the timeframe/consequences ?
(In reply to Andrew Benton from comment #20)
> Created attachment 611180 [details] [diff] [review]
> patch the new functions into cairo-1.12.0
> 
> Improved patch for cairo-1.12.0. This patch works in that I can compile
> Firefox with --enable-system-cairo. cairo-1.12.0 is pretty buggy though
> https://bugs.freedesktop.org/show_bug.cgi?id=47266
> https://bugs.gentoo.org/show_bug.cgi?id=409593

Note that the bugs lie on the X server side, not on the cairo side. Which means that whenever we upgrade cairo in the mozilla codebase, we'll hit those X server bugs :-/
(In reply to Landry Breuil from comment #22)
> Jeff,
> you're saying "our reliance on this API should be temporary", what's the
> timeframe/consequences ?

Basically, we need to port the some of the image code to use Azure directly. Once, that happens we won't need to use private APIs anymore and can re-enable system-cairo support. I'm not sure if we'll be able to do this before we switch everything over to Azure or not. The timeframe depends largely on this.

Comment 25

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> System-cairo is not really a supported configuration
Wow, and I thought it was only Chromium guys being crazy enough to fork anything they touch with Firefox team being more prudent in terms of effort and output... hope it's still so.

Comment 26

5 years ago
The breakage of system-cairo support breaks out GTK3 Firefox port too, because it requires to use system cairo (all GTK3 widgets are rendered by system cairo). Can we help somehow with the changes mentioned in comment 24? "to port the some of the image code to use Azure directly".

Comment 27

5 years ago
Anyway, there's another regression with --enable-system-cairo:

gmake[6]: Entering directory `/home/komat/tmp646-trunk/src/objdir/layout/media'
/home/komat/tmp646-trunk/src/config/rules.mk:745: *** SHARED_LIBRARY_LIBS must contain .a files only.  Stop.

because with --enable-system-cairo the SHARED_LIBRARY_LIBS contains cairo dynamic libraries.
(In reply to Martin Stránský from comment #27)
> Anyway, there's another regression with --enable-system-cairo:
> 
> gmake[6]: Entering directory
> `/home/komat/tmp646-trunk/src/objdir/layout/media'
> /home/komat/tmp646-trunk/src/config/rules.mk:745: *** SHARED_LIBRARY_LIBS
> must contain .a files only.  Stop.
> 
> because with --enable-system-cairo the SHARED_LIBRARY_LIBS contains cairo
> dynamic libraries.

It's the same as bug 751521. You may want to kill the two birds with the same stone.

Comment 29

5 years ago
Created attachment 624394 [details] [diff] [review]
Make system cairo work again

This patch makes mozilla build here with system cairo again. It replaces usage of cairo_surface_attach_snapshot() and _create_nil_surface() with public cairo API (cairo_surface_set_mime_data()). Mime data gets detached at the same time that snapshots get detached (see _cairo_surface_begin_modification()).
Attachment #624394 - Flags: review?(bas.schouten)

Comment 30

5 years ago
Comment on attachment 624394 [details] [diff] [review]
Make system cairo work again

Review of attachment 624394 [details] [diff] [review]:
-----------------------------------------------------------------

As I see, your patch breaks the correct rendering of the following page (press End): http://rohejorszaga.blog.hu/2012/06/03/erre_egyszeruen_nem_talalok_szavakat_601?fullcommentlist=1#comments

Comment 31

5 years ago
@comment 30:
Well, it seems even holding PageDown for awhile triggers the problem (it probably reverts to the state of bug 715658).
So, anyone up to asking *cairo* upstream how would they solve the original problem within existing *public* API ?
Or is the attachment 624394 [details] [diff] [review] the correct solution and it's firefox, that needs a bit stricter resource management ?
(In reply to Rafał Mużyło from comment #31)
> @comment 30:
> Well, it seems even holding PageDown for awhile triggers the problem (it
> probably reverts to the state of bug 715658).
> So, anyone up to asking *cairo* upstream how would they solve the original
> problem within existing *public* API ?
> Or is the attachment 624394 [details] [diff] [review] the correct solution
> and it's firefox, that needs a bit stricter resource management ?

I don't know what specific version of firefox and cairo you use, but it works fine for me with cairo 1.12 and firefox 14. However, cairo 1.12 is known to expose X.org driver problems.

Comment 33

5 years ago
@comment 30: most of it is a miss.
I *am* using cairo 1.12, but I'm also using the drivers with the fix for that problem (or at least that part of it that was easily exposed in firefox - basic acceleration of solid pictures via scratch 1x1 pixmaps). I'm using (as of yesterday) firefox 13 though, so perhaps it's fixed in 14 already.

Well, that is unless you're talking about a different set of cairo/X driver problems.
(In reply to Rafał Mużyło from comment #33)
> @comment 30: most of it is a miss.
> I *am* using cairo 1.12, but I'm also using the drivers with the fix for
> that problem (or at least that part of it that was easily exposed in firefox
> - basic acceleration of solid pictures via scratch 1x1 pixmaps). I'm using
> (as of yesterday) firefox 13 though, so perhaps it's fixed in 14 already.

Works fine here in firefox 13 as well.

> Well, that is unless you're talking about a different set of cairo/X driver
> problems.

AFAIK, there are several different problems, not all of which are fixed ATM.

Comment 35

5 years ago
(In reply to Mike Hommey [:glandium] from comment #34)
> (In reply to Rafał Mużyło from comment #33)
> > Well, that is unless you're talking about a different set of cairo/X driver
> > problems.
> 
> AFAIK, there are several different problems, not all of which are fixed ATM.

Can you share the bug numbers in upstream bugzilla or ... ?

Comment 36

5 years ago
(In reply to Rafał Mużyło from comment #35)
> (In reply to Mike Hommey [:glandium] from comment #34)
> > (In reply to Rafał Mużyło from comment #33)
> > > Well, that is unless you're talking about a different set of cairo/X driver
> > > problems.
> > 
> > AFAIK, there are several different problems, not all of which are fixed ATM.
> 
> Can you share the bug numbers in upstream bugzilla or ... ?

I did in comment #20

Comment 37

5 years ago
(In reply to Andrew Benton from comment #36)
> (In reply to Rafał Mużyło from comment #35)
> > Can you share the bug numbers in upstream bugzilla or ... ?
> 
> I did in comment #20
Your info is out of date (and there were a few of my comments in both of those bugs).
The Gentoo bug was exactly the same problem as the freedesktop one and the later one has long been fix in all three major driver branches (intel, ati and nouveau).

Comment 38

5 years ago
Created attachment 630531 [details]
minimal testcase - html

...and on a related note: I do notice an odd case of display corruption, though I'm not quite sure whether it's related to cairo 1.12.

I've managed to reduce it into a nearly trivial test case.
To trigger the corruption put html and css files in one folder, and hover the mouse over those two links. As you switch a few times between them, backrground of the links becomes corrupted.

Comment 39

5 years ago
Created attachment 630532 [details]
minimal testcase - css
(In reply to Rafał Mużyło from comment #38)
> Created attachment 630531 [details]
> minimal testcase - html
> 
> ...and on a related note: I do notice an odd case of display corruption,
> though I'm not quite sure whether it's related to cairo 1.12.
> 
> I've managed to reduce it into a nearly trivial test case.
> To trigger the corruption put html and css files in one folder, and hover
> the mouse over those two links. As you switch a few times between them,
> backrground of the links becomes corrupted.

Still no problem here. Can we just stop chasing cairo or Xorg bugs here anyways?

Comment 41

5 years ago
Created attachment 630538 [details]
minimal testcase - html (corrected)

Minor correction of the testcase (now it validates as xhtml).
Attachment #630531 - Attachment is obsolete: true

Comment 42

5 years ago
This bug became a bit of a mess, so lets summarize:
While using patch from comment 29:
- that link problem happens only in cairo built with '--enable-xlib-xcb', so my mistake there (that option is disabled by default)
- problem in bug 715658 happens regardless of cairo version, so patch might be incomplete
- problem from comment 30 happens only with cairo 1.12 - I'll see what upstream bugzilla has to say
(In reply to Rafał Mużyło from comment #42)
> - problem in bug 715658 happens regardless of cairo version, so patch might
> be incomplete

With https://bugzilla.mozilla.org/attachment.cgi?id=624394, it doesn't. The patch is no incomplete.

Comment 44

5 years ago
Created attachment 630607 [details]
a screenshot of the site

With both cairo 1.10.2 and 1.12.2, http://scoop.today.msnbc.msn.com/_news/2012/01/05/9977921-rooney-mara-girl-with-the-style-we-want-to-steal looks like on the attached screenshot while using the mentioned patch.

I don't think that black rectangle is the correct display.
(In reply to Rafał Mużyło from comment #44)
> Created attachment 630607 [details]
> a screenshot of the site
> 
> With both cairo 1.10.2 and 1.12.2,
> http://scoop.today.msnbc.msn.com/_news/2012/01/05/9977921-rooney-mara-girl-
> with-the-style-we-want-to-steal looks like on the attached screenshot while
> using the mentioned patch.
> 
> I don't think that black rectangle is the correct display.

And again, this has nothing to do with this bug, and works fine here. Can we stop here?
(Assignee)

Comment 46

5 years ago
Created attachment 648007 [details] [diff] [review]
Updated patch

Guys, can it be committed?
Attachment #648007 - Flags: review?(bas.schouten)
(Assignee)

Comment 47

5 years ago
ping?

Comment 48

5 years ago
Every EXA driver is different but in my case, text looks much better with --enable-system-cairo. If a program is designed to use a version of cairo that has 91 patches applied, this doesn't seem right to me.
(Assignee)

Comment 49

5 years ago
Four month since there is a working patch with review request, even longer build with system cairo is broken.
Can anybody review/commit the patch?

Comment 50

5 years ago
This patch is being pushed by people who think it is inefficient for mozilla to try to fork cairo. However, mozilla has now decided to go with the even more inefficient route of writing a vector graphics library from scratch to replace cairo. Maybe that's why they aren't committing this patch.

Updated

5 years ago
Blocks: 627699
(Assignee)

Updated

5 years ago
Blocks: 795737

Comment 51

5 years ago
Also status "UNCONFIRMED" is obviously false...
(Assignee)

Comment 52

5 years ago
Created attachment 669619 [details] [diff] [review]
Updated patch

And, after nine month since the bug was filled, the patch has bitrotted once again.
:/
Attachment #648007 - Attachment is obsolete: true
Attachment #648007 - Flags: review?(bas.schouten)
Attachment #669619 - Flags: review?(bas.schouten)
Comment on attachment 669619 [details] [diff] [review]
Updated patch

Review of attachment 669619 [details] [diff] [review]:
-----------------------------------------------------------------

If this works lets land it. We need to keep a very close eye if it doesn't break anything though.
Attachment #669619 - Flags: review?(bas.schouten) → review+
(Assignee)

Comment 54

5 years ago
Hooray!
Keywords: checkin-needed
I don't see any Try results here, so I've gone ahead and started a run myself.
https://tbpl.mozilla.org/?tree=Try&rev=034a386f0a57

Uli, sorry for the extra delay, but given Bas' comments, I want to make sure it at least passes Mozilla's internal testsuite before pushing it. I will push it as soon as the Try comes back OK.
Green on Try.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2231b8e778a3

Uli, thanks again for the patch! Just so you know where things are in the process, this patch has landed on mozilla-inbound, which is an intermediate integration branch. It is merged to mozilla-central (the official repo) once or twice daily. Once it lands on m-c, this bug will be resolved as FIXED. At this point, it will land for Firefox 19. If you want, you can request approval on the patch for aurora/beta to get it in an earlier release, but I can't promise you that the request will be accepted. Thanks again!
Assignee: bas.schouten → ojab
Flags: in-testsuite-
Keywords: checkin-needed
(In reply to Ryan VanderMeulen from comment #56)
> Green on Try.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/2231b8e778a3

Unfortunately this just tested Linux & this caused failures on other platforms, so I've had to back it out.

Windows doesn't compile with this patch - linking errors, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=15982013&tree=Mozilla-Inbound

Tests on OS X crash, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=15982181&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=15982231&tree=Mozilla-Inbound

Full results:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=dee0f227e656

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fa702386c7c

Comment 58

5 years ago
No clue what the heck is wrong on windows. cairo-rename.h handles cairo_surface_set_mime_data() the same way it handles other cairo functions and there is nothing windows-specific in there. So why does this only happen on windows?

For the OS X crashes:
No good idea here either. The stack trace looks bogous. _cairo_mime_data_destroy is supposed to call the function pointer given to cairo_surface_set_mime_data(), so this is supposed to end up in SourceSnapshotDetached. However, for some weird reason, _cairo_user_data_array_set_data() is called instead. Weird memory corruption?
(Assignee)

Comment 59

5 years ago
Looks like windows build is broken because of https://hg.mozilla.org/integration/mozilla-inbound/rev/27c51f14c3ed
https://hg.mozilla.org/mozilla-central/rev/05006509aa53
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
This was backed out, no?
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Target Milestone: mozilla19 → ---
Ah crap, that changeset had the wrong bug number in it. The changeset in comment #60 belongs to bug 779209.
(Assignee)

Updated

5 years ago
Blocks: 798926
Comment on attachment 624394 [details] [diff] [review]
Make system cairo work again

Review of attachment 624394 [details] [diff] [review]:
-----------------------------------------------------------------

Afaiui this doesn't work yet.
Attachment #624394 - Flags: review?(bas)

Comment 64

4 years ago
Which build is broken on Windows? The --enable-system-cairo one that didn't work before or --disable-system-cairo? If it's the latter, the problem can still be fixed temporarily using ifdefs right? This would beat having to wait hours for a -Wl,--no-keep-memory build to finish.

Comment 65

4 years ago
I's like bring a little corner case to your attention, that comes from using patch from attachment 624394 [details] [diff] [review] - bug 862422. It might be hard to tell whether the problem lies on cairo or libxul side. Also, the conditions to trigger the case are very specific, but it's not being established what they exactly are.

Comment 66

4 years ago
Created attachment 753419 [details] [diff] [review]
Patch with ifdefs
Attachment #753419 - Flags: review?

Comment 67

4 years ago
(In reply to Connor Behan from comment #66)
> Created attachment 753419 [details] [diff] [review]
> Patch with ifdef

The ifdefs should not be against linux but rather against MOZ_TREE_CAIRO.
Comment on attachment 753419 [details] [diff] [review]
Patch with ifdefs

Reviews without a person attached to them are likely to be ignored, best to choose someone, even if you aren't sure they can redirect the review request.
Attachment #753419 - Flags: review? → review?(bas)

Comment 69

4 years ago
Created attachment 753610 [details] [diff] [review]
Patch with ifdefs

Yeah, that makes more sense.
Attachment #753419 - Attachment is obsolete: true
Attachment #753419 - Flags: review?(bas)
Attachment #753610 - Flags: review?(bas)
Attachment #753610 - Flags: review?(bas) → review+

Comment 70

4 years ago
(In reply to Connor Behan from comment #69)
BTW, switching by tree or not is a good idea?
If this is depended by the version of cairo, we should use it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/52b02042b27f
https://hg.mozilla.org/mozilla-central/rev/52b02042b27f
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24

Updated

3 years ago
Duplicate of this bug: 880216

Updated

3 years ago
Blocks: 1034064

Updated

3 years ago
No longer blocks: 1034064
You need to log in before you can comment on or make changes to this bug.