Closed Bug 562746 Opened 14 years ago Closed 13 years ago

Update cairo to 1.10

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7
Tracking Status
blocking2.0 --- -

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

(Depends on 8 open bugs, Blocks 1 open bug)

Details

Attachments

(7 files, 18 obsolete files)

1.95 MB, patch
Details | Diff | Splinter Review
966 bytes, patch
Details | Diff | Splinter Review
394 bytes, text/html
Details
1.06 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.91 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
2.04 MB, patch
Details | Diff | Splinter Review
1.43 KB, patch
Details | Diff | Splinter Review
This update should get us up-to-date with last cairo trunk
Blocks: 545632
Attached patch First try (obsolete) — Splinter Review
Blocks: 526977
This gives us two failures on OS X:
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/smil/sort/sort-additive-1.svg |
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/text-font-weight-01.svg |
Attached patch v2 (obsolete) — Splinter Review
Fixes some d2d compilation problems and updates to cairo trunk
Attachment #443664 - Attachment is obsolete: true
Blocks: 563255
Attached patch v3 (obsolete) — Splinter Review
More updates. Doesn't build on win32 for some reason.
Attachment #444016 - Attachment is obsolete: true
Blocks: 569669
(In reply to comment #2)
> This gives us two failures on OS X:
> REFTEST TEST-UNEXPECTED-FAIL |
> file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/smil/sort/sort-additive-1.svg
> |

Are you sure this one isn't just random orange i.e. bug 547801
I'm seeing some new weird failures on OS X. I get aliased fonts for some things on tests like:

layout/reftests/bugs/179596-2.html
 > layout/reftests/bugs/179596-2.html

Can we just push new cairo update, and record all known issues as bugs ?
Not really, it will cause a bunch of orange. Help fixing the issues would be great though :)
Blocks: 566283
(In reply to comment #7)
> I'm seeing some new weird failures on OS X. I get aliased fonts for some things
> on tests like:
> 
> layout/reftests/bugs/179596-2.html

Looks like this is a bug with the state management of CGContextSetShouldAntialias
Blocks: 583135
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t

>+static cairo_status_t
>+_cairo_image_surface_fixup_unbounded (cairo_image_surface_t *dst,
>+				      const cairo_composite_rectangles_t *rects,
>+				      cairo_clip_t *clip)
>+{
>+    pixman_image_t *mask = NULL;
>+    pixman_box32_t boxes[4];
>+    int i, mask_x = 0, mask_y = 0, n_boxes = 0;
>+
>+    if (clip != NULL) {
>+	cairo_surface_t *clip_surface;
>+	int clip_x, clip_y;
>+
>+	clip_surface = _cairo_clip_get_surface (clip, &dst->base, &clip_x, &clip_y);
>+	if (unlikely (clip_surface->status))
>+	    return clip_surface->status;
>+
>+	mask = ((cairo_image_surface_t *) clip_surface)->pixman_image;
>+	mask_x = -clip_x;
>+	mask_y = -clip_y;
>+    } else {

Coverity complains that this leaks clip_surface when clip_surface->status is not a failure. Yet pixman_image is rescued, but its container is leaked.
Attachment #447523 - Flags: feedback-
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t


>+static cairo_status_t
>+_composite_boxes (cairo_image_surface_t *dst,
>+		  cairo_operator_t op,
>+		  const cairo_pattern_t *pattern,
>+		  cairo_boxes_t *boxes,
>+		  cairo_antialias_t antialias,
>+		  cairo_clip_t *clip,
>+		  const cairo_composite_rectangles_t *extents)
>+{
>+    cairo_region_t *clip_region = NULL;
>+    cairo_bool_t need_clip_mask = FALSE;
>+    cairo_status_t status;
>+    struct _cairo_boxes_chunk *chunk;
>+    uint32_t pixel;
>+    int i;
>+
>+    if (clip != NULL) {
>+	status = _cairo_clip_get_region (clip, &clip_region);
>+	need_clip_mask = status == CAIRO_INT_STATUS_UNSUPPORTED;
>+	if (need_clip_mask &&
>+	    (op == CAIRO_OPERATOR_SOURCE || ! extents->is_bounded))
>+	{
...
>+	}
>+
>+	if (clip_region != NULL && cairo_region_num_rectangles (clip_region) == 1)
>+	    clip_region = NULL;
>+    }
>+
>+    if (antialias != CAIRO_ANTIALIAS_NONE) {
...
>+    }
>+
>+    status = CAIRO_STATUS_SUCCESS;
>+    if (! need_clip_mask &&
>+	pattern_to_pixel ((cairo_solid_pattern_t *) pattern, op, dst->pixman_format,
>+			  &pixel))
>+    {
...
>+    }
>+    else
>+    {
mask is initialized to NULL:
>+	pixman_image_t *src = NULL, *mask = NULL;
...

let need_clip_mask be false:
>+	if (need_clip_mask) {
...
>+	}
>+

let pattern be false:
>+	if (pattern != NULL) {
...
>+	} else {

src is now null because mask was null:
>+	    src = mask;
...
>+	    mask = NULL;
>+	}
>+
>+	for (chunk = &boxes->chunks; chunk != NULL; chunk = chunk->next) {
>+	    const cairo_box_t *box = chunk->base;
>+
>+	    for (i = 0; i < chunk->count; i++) {
>+		int x1 = _cairo_fixed_integer_round (box[i].p1.x);
>+		int y1 = _cairo_fixed_integer_round (box[i].p1.y);
>+		int x2 = _cairo_fixed_integer_round (box[i].p2.x);
>+		int y2 = _cairo_fixed_integer_round (box[i].p2.y);
>+
>+		if (x2 == x1 || y2 == y1)
>+		    continue;

here src and mask are passed as null:
>+		pixman_image_composite32 (pixman_op,
>+                                          src, mask, dst->pixman_image,
>+                                          x1 + src_x,  y1 + src_y,
>+                                          x1 + mask_x, y1 + mask_y,
>+                                          x1, y1,
>+                                          x2 - x1, y2 - y1);

And we crash under _pixman_image_validate (src);
Attachment #447523 - Flags: feedback-
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t

+static cairo_status_t
+_clip_and_composite (cairo_image_surface_t	*dst,
+		     cairo_operator_t		 op,
+		     const cairo_pattern_t	*src,
+		     image_draw_func_t		 draw_func,
+		     void			*draw_closure,
+		     cairo_composite_rectangles_t*extents,
+		     cairo_clip_t		*clip)
+{
+    cairo_status_t status;
+    cairo_region_t *clip_region = NULL;
+    cairo_bool_t need_clip_surface = FALSE;
+
let clip be null:
+    if (clip != NULL) {
...
+    }
+
let clip_region be null:
+    if (clip_region != NULL) {
...
+    }
+
let this function return false:
+    if (reduce_alpha_op (dst, op, src)) {
...
+    }
+
let op be CAIRO_OPERATOR_SOURCE:
+    if (op == CAIRO_OPERATOR_SOURCE) {
clip is null and we crash in here:
+	status = _clip_and_composite_source (clip, src,
+					     draw_func, draw_closure,
+					     dst, &extents->bounded);
Attachment #447523 - Flags: feedback-
Summary: Update cairo to upstream → Update cairo to 1.10
Can you also, as part of this, normalize line endings in the d2d code?  That is, remove all the dos line endings...
(In reply to comment #15)
> Can you also, as part of this, normalize line endings in the d2d code?  That
> is, remove all the dos line endings...

I'll do that separately.
This seems to be causing intermittent failures of 

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/tryserver_fedora64-debug_test-reftest/build/reftest/tests/layout/reftests/svg/smil/sort/sort-additive-1.svg |

The failures seem clipping unrelated and perhaps not the same as bug 547801
Attachment #447523 - Attachment is obsolete: true
> >+
> >+	clip_surface = _cairo_clip_get_surface (clip, &dst->base, &clip_x, &clip_y);
> >+	if (unlikely (clip_surface->status))
> >+	    return clip_surface->status;
> >+
> >+	mask = ((cairo_image_surface_t *) clip_surface)->pixman_image;
> >+	mask_x = -clip_x;
> >+	mask_y = -clip_y;
> >+    } else {
> 
> Coverity complains that this leaks clip_surface when clip_surface->status is
> not a failure. Yet pixman_image is rescued, but its container is leaked.

The surface is owned by the clip_path and destroyed in _cairo_clip_path_destroy
(In reply to comment #13)
> Comment on attachment 447523 [details] [diff] [review]
> v3

> 
> let pattern be false:

I'm don't think there are cases where pattern is NULL
(In reply to comment #14)
> Comment on attachment 447523 [details] [diff] [review]
> v3

> let op be CAIRO_OPERATOR_SOURCE:
> +    if (op == CAIRO_OPERATOR_SOURCE) {
> clip is null and we crash in here:
> +    status = _clip_and_composite_source (clip, src,
> +                         draw_func, draw_closure,
> +                         dst, &extents->bounded);

_clip_and_composite_source seems to deal with a NULL clip, so I'm not sure why we'd crash.
Karl,

Any guesses why we could get this kind of rendering: http://pastebin.mozilla.org/824349 ?
No ideas, sorry.
Is the assumption that the similar failure in comment 2 was bug 547801, so the new issue is fedora 64 specific?  or intermittent?
Looks like it was caused by:

commit 6b77567b6ef28710c7707ab82c7fa95c810152d1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 22 15:54:45 2010 +0000

    path: Compute coarse bounds upon construction.
    
    Frequently we only need the coarse path bounds, so avoid walking over
    the list of points once more as we can cheaply track the extents during
    construction.
and should be fixed with:

diff --git a/src/cairo-path-bounds.c b/src/cairo-path-bounds.c
index 8f9bdc3..d46b8e5 100644
--- a/src/cairo-path-bounds.c
+++ b/src/cairo-path-bounds.c
@@ -331,7 +331,7 @@ _cairo_path_fixed_extents (const cairo_path_fixed_t *path,
 
     if (! path->has_curve_to) {
 	*box = path->extents;
-	return path->extents.p1.x < path->extents.p2.x;
+	return path->extents.p1.x <= path->extents.p2.x;
     }
 
     _cairo_path_bounder_init (&bounder);
It seems like 3b1c0a4bd66660780095e6016e3db451f34503a3 is the other commit that broke a win32 test.
This could be a lot of work to fix...
Attached patch v5 (obsolete) — Splinter Review
Attachment #485329 - Attachment is obsolete: true
Blocks: 601512
blocking2.0: --- → ?
Right now I don't think we absolutely require Cairo 1.10. If it becomes a hard blocker for anyone, this could change.
blocking2.0: ? → -
Depends on: 616700
(In reply to comment #29)
> Right now I don't think we absolutely require Cairo 1.10. If it becomes a hard
> blocker for anyone, this could change.

requiring cairo 1.10 will require update libpixman-1 (which is easy) and also require updating glib-2.0 for CentOS5/RHEL5/ScienticLinux.
Building cairo 1.10 requires at least Glib 2.14.

Dropping support for a widespread used Enterprise Linux distro isn't a good idea or at least Firefox 3.6 should get security fixes for 3 more years until RHEL5 is EoL ;-)
on www.ask.com Fennec show partially black  screen with cairo 1.10
Bug 601512 is a hardblocker that says it will be fixed by this bug. I guess this needs to hardblock as well, correct?
Blocks: 629012
Would this also repair bug 595671, based on comments in that bug?
Applying to m-c 716b7303beea gives

applying imp-562746-cairo-1.10
patching file gfx/cairo/cairo/src/Makefile.in
Hunk #3 FAILED at 270
1 out of 3 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/Makefile.in.rej
patching file gfx/cairo/cairo/src/cairo-d2d-private.h
Hunk #1 FAILED at 37
1 out of 1 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-d2d-private.h.rej
patching file gfx/cairo/cairo/src/cairo-d2d-surface.cpp
Hunk #1 FAILED at 33
Hunk #2 FAILED at 624
Hunk #3 FAILED at 710
Hunk #4 FAILED at 1388
Hunk #5 FAILED at 1435
Hunk #6 FAILED at 1467
Hunk #7 FAILED at 1572
Hunk #8 FAILED at 2251
Hunk #9 FAILED at 2367
Hunk #10 FAILED at 2440
Hunk #11 FAILED at 2512
Hunk #12 FAILED at 3120
Hunk #13 FAILED at 3212
Hunk #14 FAILED at 3391
Hunk #15 FAILED at 3447
Hunk #16 FAILED at 3558
Hunk #17 FAILED at 3587
Hunk #18 FAILED at 3672
Hunk #19 FAILED at 3727
Hunk #20 FAILED at 3773
Hunk #21 FAILED at 3796
Hunk #22 FAILED at 3856
22 out of 22 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-d2d-surface.cpp.rej
patching file gfx/cairo/cairo/src/cairo-gstate.c
Hunk #22 FAILED at 1956
1 out of 25 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-gstate.c.rej
patching file gfx/cairo/cairo/src/cairo-qt-surface.cpp
Hunk #7 FAILED at 1601
1 out of 8 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-qt-surface.cpp.rej
patching file gfx/cairo/cairo/src/cairo-quartz-surface.c
Hunk #4 FAILED at 117
Hunk #5 FAILED at 149
Hunk #20 FAILED at 1762
Hunk #27 FAILED at 2402
Hunk #30 FAILED at 2855
5 out of 35 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-quartz-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-surface-private.h
Hunk #2 FAILED at 35
1 out of 2 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-surface-private.h.rej
patching file gfx/cairo/cairo/src/cairo-surface.c
Hunk #2 FAILED at 34
Hunk #5 FAILED at 208
2 out of 37 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-win32-surface.c
Hunk #4 FAILED at 116
1 out of 12 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-win32-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-xlib-surface.c
Hunk #49 FAILED at 3820
Hunk #50 FAILED at 3845
Hunk #51 FAILED at 3914
Hunk #66 succeeded at 4682 with fuzz 1 (offset 10 lines).
Hunk #68 FAILED at 4734
4 out of 70 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-xlib-surface.c.rej
patching file gfx/cairo/cairo/src/cairo.h
Hunk #6 FAILED at 2065
Hunk #7 FAILED at 2114
Hunk #9 FAILED at 2216
3 out of 14 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo.h.rej
patching file gfx/cairo/cairo/src/cairoint.h
Hunk #29 FAILED at 2477
1 out of 30 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairoint.h.rej
patching file gfx/thebes/gfxASurface.cpp
Hunk #1 FAILED at 475
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxASurface.cpp.rej
patching file gfx/thebes/gfxASurface.h
Hunk #1 FAILED at 90
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxASurface.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh imp-562746-cairo-1.10

I'll try to update what I can to test bug 637828.
Thankfully Timothy had a fairly recent rebase that made this much easier.

Fixes bug 637828.
Blocks: 610344
This makes some graphics corruptions issues on the start page worse, but fixes a lot of our other bugs. We'll need to see what's going on there. I think Fennec does need this for better or for worse.
to add to what stechz said, two bugs marked blocking-fennec are solved by this update, requesting blocking-fennec for this bug so it can be triaged
No longer blocks: 616638
tracking-fennec: --- → ?
The path of least resistance (although also least elegance) to using cairo 1.10(.2?) and pixman 0.21(.6?) for fennec would be
 - copy vanilla releases into gfx/cairo/new-cairo or something like that.
 - apply pixman-android-cpu-detect patch.  There might be others we need.
 - copy build goop from gfx/cairo/cairo/src to gfx/cairo/new-cairo/src
 - update for build goop for cairo+pixman changes
 - split off the gecko (non-cairo) parts of Jeff's patch here
 - add a new --enable-new-cairo configure flag that selects new-cairo

new-cairo can be built, linked, and shipped in libxul like the current in-tree cairo.  I would guess this is a full day's worth of work for someone familiar with the build system (e.g., me).

Part of the decision to take this is the value in fixing the dependent graphical bugs, which is mostly known already.  The other part of the decision is the perf difference with latest cairo+pixman.  We won't have an accurate estimate of perf until we do all the import+build work.  It would normally be possible to build --enable-system-cairo/--enable-system-pixman and package the .so's with fennec, but I imagine that porting both to android might be as much or more work than just importing both into our tree.
I sent off a try run (a8e04b6d8135) with the patch here to get a rough idea of perf difference.  Very rough because the runs will still use older in-tree pixman and the update here to a not-latest cairo.
cjones, can you file a bug for taking that subset of patches? and leave this one for the full update.
tracking-fennec: ? → 2.0+
Yep, was just about to do that.
Going to move the fennec deps here to bug 638594, apologies for bug spam.
removing the blocking flag from this and adding it to the bug that cjones just filed
tracking-fennec: 2.0+ → ---
Attached patch Cairo Qt fix (obsolete) — Splinter Review
Attachment #448992 - Attachment is obsolete: true
Comment on attachment 516749 [details] [diff] [review]
Cairo Qt fix


oh, /gfx/thebes/gfxQPainterSurface.cpp - part is wrong
Attached patch Cairo Qt fix v2Splinter Review
this compiles fine
Attachment #516749 - Attachment is obsolete: true
Comment on attachment 516549 [details] [diff] [review]
Updated to m-c 716b7303beea

I'm not 100% confident in the rebasing I did, but it was enough to get it working and to test it on top of (what was then) current trunk. In case anyone was thinking of committing this somewhere...
That's OK, I'm not 100% confident in my rebasing either :).  This was more for proof-of-concept purposes.
Attached patch rebased on top of 4e771e65764a (obsolete) — Splinter Review
Attachment #490975 - Attachment is obsolete: true
Comment on attachment 519213 [details] [diff] [review]
rebased on top of 4e771e65764a

The added files are missing in this patch.
This one is more likely to build.
Attachment #519213 - Attachment is obsolete: true
I'm seeing a reftest failure on OS X on test bugs/615121-1.html
(In reply to comment #54)
> I'm seeing a reftest failure on OS X on test bugs/615121-1.html

This is caused by 9c0d761bfcdd28d52c83d74f46dd3c709ae0fa69 which should be pretty easy to fix.
Attached patch rebased on top of 4f07bebb993b (obsolete) — Splinter Review
Attachment #519411 - Attachment is obsolete: true
This fixes the quartz issue and adds a buggy version of gdi changes.
Attachment #519737 - Attachment is obsolete: true
an alternative approach to fallback:
http://people.mozilla.com/~jmuizelaar/cairo-update-march
Blocks: 614721
Seems like there's a memory leak when running reftests with d2d.
Attached patch rebased again (obsolete) — Splinter Review
Attachment #521189 - Attachment is obsolete: true
Attached patch Rebased (for real this time) (obsolete) — Splinter Review
Attachment #531318 - Attachment is obsolete: true
Attached file Rendering bug testcase
I found this bug when testing Jeff's patch on Wikipedia.

Try zooming in on this test case.  A vertical line appears at the boundaries of the <a> elements in some zoom levels.
Depends on: 656465
The known issues left are:
- d2d leak
- mac image drawing bug that ehsan posted about
Attachment #531400 - Attachment is obsolete: true
The newer quartz backend actually supports extend none properly. We currently (and in the update) patch extend_pad to behave like the old extend_none instead of a real extend_pad, so this change shouldn't have any rendering differences now and fixes the reftest when we update.
Attachment #532207 - Flags: review?(roc)
Attachment #532241 - Attachment is obsolete: true
Attachment #532243 - Flags: review?(ehsan)
Attachment #532241 - Flags: review?(ehsan)
Comment on attachment 532243 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10

Please specify the line-height explicitly, because it may change because of any number of reasons.  Also, please make sure that the test works fine on Android before landing it (and if it doesn't, you can just tag it with fails-if).

r=me with that.
Attachment #532243 - Flags: review?(ehsan) → review+
Comment on attachment 532207 [details] [diff] [review]
Remove use of EXTEND_NONE on quartz

Review of attachment 532207 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #532207 - Flags: review?(roc) → review+
Attached patch rebased on top 1f3777d4ed8b (obsolete) — Splinter Review
Attachment #531780 - Attachment is obsolete: true
I just noticed that there are a bunch of android reftest failures caused by this. They seem to be largely caused by the difference between drawing things like: [q][uestions] and [questions]. I'm not sure why that's happening yet though.
This fixes most of the android reftest failures. There are two left:
generated-content/display-types-01.html
counters/counter-reset-integer-range.html
Attachment #533382 - Attachment is obsolete: true
Attached patch rebased againSplinter Review
Most things are good with this version but I intermittently get:

TEST-PASS | chrome://mochitests/content/browser/toolkit/components/console/hudservice/tests/browser/browser_webconsole_bug_613642_maintain_scroll.js | scroll location is not at the top
TEST-PASS | chrome://mochitests/content/browser/toolkit/components/console/hudservice/tests/browser/browser_webconsole_bug_613642_maintain_scroll.js | scroll location updated (moved to top)
###!!! ASSERTION: recursive painting not permitted: '!IsPainting()', file 

command timed out: 1200 seconds without output, killing pid 3440
Attachment #533949 - Attachment is obsolete: true
Fixed it. It was a problem of allocating using a different type than it actually was.
Sorry to spam this bug, but could bug #659676 be related to this one ?
Depends on: 659676
Backed out in http://hg.mozilla.org/mozilla-central/rev/a98c00e70590 because of possible Mac OS X 10.5 Dromaeo regression.

I'll reland tomorrow if this regression proves to not be the fault of this patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
I see bugzilla is still seriouly F*ucked up...all I did was CC myself and the stupid bug closed...

reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(yeah, that bugzilla quirk is a known issue.  basically, if your bug tweak midair-collides with a change to any other fields, then your commit will set those fields back to their old values, unless you shift-reload.  Usually the "midair collision" splash page is a good heads-up that you should go back and shift-reload. It sucks, I know.)
I don't know what shift-reload does, but normal reload is bad because remembers all the form settings and reapplies them after someone has changed any.
Relanded:

http://hg.mozilla.org/mozilla-central/rev/877f11b44bfe
http://hg.mozilla.org/mozilla-central/rev/05b2836292b2
http://hg.mozilla.org/mozilla-central/rev/102be3d1f103
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Depends on: 660448
Blocks: 646611
Depends on: 456448
Attached patch Delete some code we don't need (obsolete) — Splinter Review
Attachment #536685 - Attachment is obsolete: true
Assignee: nobody → jmuizelaar
The cairo version in cairo-features.h.in is still 1.9.5. The version in gfx/cairo/README has also not been updated.
Depends on: 668549
Depends on: pixman-coord
Depends on: 689217
Depends on: 689962
Depends on: 691167
Depends on: 692350
Blocks: 404637
Depends on: 698007
Depends on: 700003
Depends on: 716462
Depends on: 720035
Depends on: 725937
Depends on: 730571
Depends on: 733714
Blocks: 567370
Depends on: 801794
Depends on: 808249
Depends on: 768362
Depends on: 873313
Depends on: 877492
Depends on: 896537
Depends on: 886642
No longer depends on: 886642
Depends on: 974310
Depends on: 989592
Comment on attachment 534865 [details] [diff] [review]
rebased again

>-			 (LPSTR) &lpMsgBuf,
>+			 (LPWSTR) &lpMsgBuf,
> 			 0, NULL)) {
> 	fprintf (stderr, "%s: Unknown GDI error", context);
>     } else {
>-	fprintf (stderr, "%s: %S", context, (char *)lpMsgBuf);
>+	fwprintf (stderr, L"%s: %S", context, (wchar_t *)lpMsgBuf);
So, this line originally started out as fprintf (stderr, "%s: %s", context, lpMsgBuf); where the message was retrieved using FormatMessageA and was therefore in ANSI.
Bug 458496 then switched to using FormatMessageW and fwprintf and used "%S: %s" as the fwprintf format. However this should have been a wide string and so bug 624198 switched it back to fprintf now using "%s: %S" as the format, which does at least work.
This patch then switched back to fwprintf but only widened the format string without correcting it to L"%S: %s".
Ah, it turns out that this was ignored in bug 710976.
Depends on: 672726
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: