Small objects that are scaled up don't get invalidated correctly

RESOLVED FIXED

Status

()

Core
SVG
P2
normal
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: dholbert, Assigned: jwatt)

Tracking

({fixed1.9.1, regression})

Trunk
fixed1.9.1, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(8 attachments, 6 obsolete attachments)

478 bytes, image/svg+xml
Details
705 bytes, image/svg+xml
Details
2.04 KB, patch
Details | Diff | Splinter Review
3.31 KB, patch
longsonr
: review+
Details | Diff | Splinter Review
776 bytes, image/svg+xml
Details
8.58 KB, patch
roc
: review+
Details | Diff | Splinter Review
389 bytes, patch
dougt
: review+
Details | Diff | Splinter Review
1.09 KB, patch
longsonr
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
STR: 
 1. Install GreaseMonkey ( https://addons.mozilla.org/en-US/firefox/addon/748 )
 2. Install the FakeSmile GreaseMonkey userscript ( http://leunen.d.free.fr/fakesmile/status.html )
 3. Load URL

EXPECTED RESULTS:
 - Animated circle moving across a box.

ACTUAL RESULTS:
 - As the tiny orange circle moves across the box, it leaves a persistent trail of visual artifacts.
 - If you force a repaint by clicking the Firefox toolbar, or clicking into a different window, the existing artifacts are cleared (but new ones keep appearing)

(Note: This issue was originally noted in bug 216462 comment 130, as a potential issue with the SMIL patch.  However, given that this reproduces with the FakeSmile userscript, I think it's a repaint issue in mozilla-central somewhere)
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre ID:20081120022145
Created attachment 349240 [details]
testcase 1 (requires FakeSmile)

Here's a reduced testcase from the URL.

I think the issue has something to do with the circle's small radius -- if I increase it from 0.05 to 0.08, I get no artifacts.  (And a value of "0.075" shows very tiny artifacts)

I don't think the large transform matters, though, other than to make the testcase more visible.  (I tried removing the transform, and I could still see teeny-tiny artifacts behind the teeny-tiny circle)
Created attachment 349245 [details]
testcase 2: Just SVG and JS (no FakeSmile needed)

This testcase shows the "smeared" artifacts using a fresh profile.  (No FakeSmile / Greasemonkey required.)
Summary: SVG Repaint issue with FakeSMIL → SVG Repaint issue using JS animation
This regressed between these nightlies:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081103 Minefield/3.1b2pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081104 Minefield/3.1b2pre
Keywords: regression
Pushlog for regression range: http://tinyurl.com/5jueta
Flags: blocking1.9.1+
Created attachment 349263 [details] [diff] [review]
patch

Cairo seems to calculate the arc extents incorrectly. This applies to both stroke and path extents. Bug 459148 will remove the other Arc call in the SVG code.

canvas uses Arc and might well suffer from some effect due to this bug. Unfortunately canvas actually wants to draw an Arc at that point.

Can anyone get a regression range. It might help to report this to cairo so they can look into it.
Assignee: nobody → longsonr
Attachment #349263 - Flags: superreview?(roc)
Attachment #349263 - Flags: review?(roc)
OS: Linux → All
Hardware: PC → All
Jeff, see comment #7.
Attachment #349263 - Flags: superreview?(roc)
Attachment #349263 - Flags: superreview+
Attachment #349263 - Flags: review?(roc)
Attachment #349263 - Flags: review+
Oh you did the regression range already :-) That shows that bug 460946 exposed the underlying cairo problem. Maybe cairo has always had this issue.
Carl,

cairo_path_extents and cairo_stroke_extents should return xmax - min == ymax - ymin as there is no scaling applied when we calculate the extents of the circular arc segment. The height of the extent is approximately 10% of the correct value in this case.
cairo_arc is translated into curve_to before the extents are computed so I'm confused how the extents could be miscomputed...
It's also not necessarily desirable to switch to Ellipse() as it will only use a single Bezier curve instead of as many as needed to keep the error minimized as Arc() does.
Blocks: 460946
extents are calculated in nsSVGPathGeometryFrame::UpdateCoveredRegion. The bounds from GetUserStrokExtent and GetUserPathExtent seem to be incorrect.
What are the values being passed in path construction (for Arc() and Ellipse()) and what are the results that you expect to be getting?
In the testcase the circle has a radius of 0.05. This is what's passed into Arc. It's twice that for Ellipse as you pass in the circle's diameter. x and y vary during the test.

Looking at an example Arc...

x = 0.45, y = 0.45, r = 0.05

GetUserStrokeExtent
(x = 0.394, y = 0.445, width = 0.109, height = 0.0078)

GetUserPathExtent
(x = 0.398, y = 0.449, width = 0.101, height = 0.0000)
In _cairo_spline_decompose_into I think we're hitting the default tolerance limit which is 0.1. I don't think thebes provides a way to adjust the tolerance at the moment. I'm not sure anyway what we'd adjust the value to.
I've been able to reproduce something like with a small test app. However, I'm surprised that the error makes such a difference. In the test case we're drawing with a scale of 300, when I apply the scale before measuring the extents the error vanishes. Do we measure the extents with the scale factor applied?
We measure without the scale applied. Otherwise if you had a rotation rather than a scale there would be no way to get the correct extents. That's what bug 460946 fixed.
Keywords: checkin-needed
Whiteboard: [needs landing]
Priority: -- → P2
FWIW, see also related bug 463939 and its testcase (attachment 347196 [details]).  Its symptoms look similar to this bug, though I'm not sure if it has the same underlying root. (It definitely won't be fixed by this bug's patch, because it doesn't use any circles.)
I'll check in the patch, but can we have a test here too?
Created attachment 351032 [details] [diff] [review]
patch with reviewed ellipse change from bug 459148
Attachment #349263 - Attachment is obsolete: true
Without the ellipse change, ellipses would be affected too. The ellipse change was once part of bug 459148 but it makes more sense to land it as this bug.
Hmm, I just pushed the earlier patch to trunk as 1d84189da181.
So can you file that ellipse change as a separate patch in another bug? Thanks...
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Attachment #349263 - Attachment is obsolete: false
Attachment #351032 - Attachment is obsolete: true
Depends on: 467629
Created attachment 351045 [details] [diff] [review]
attempted reftest patch (doesn't work - passes on unpatched trunk)

(In reply to comment #20)
> I'll check in the patch, but can we have a test here too?

I don't think this bug is reftestable -- the bug is caused by us *not* repainting when we need to, but reftest can't catch that because it forces a full repaint when it takes a snapshot for comparison. (AFAIK)

I tried making a reftest just for the heck of it, but it's not good, because it passes on unpatched trunk right now (even though I can see the paint errors in the reftest window while the page laods), for the reason described above.  Here's the reftest, though, in case anyone can improve upon it.

Maybe this needs a litmus-test?
Attachment #351045 - Attachment description: attempted reftest patch (doesn't work - passes unpatched) → attempted reftest patch (doesn't work - passes on unpatched trunk)
It is not reftestable but it is mochitestable. Create an svg file with a really small circle within a <g> that magnifies it and then get its bounds. See the mochitest for bug 460946 which I've yet to fix the rounding errors in but the method is there is sound.
Daniel are you planning to do a mochitest for this?
Well, I had been intending to make a mochitest based on the one for bug 460946, as mentioned in comment 26, but I couldn't find it.  I looked at the patches attached on the bug and at the pushed patch, and I searched mozilla-central for files including "460946" in the name, but I couldn't find anything.

Maybe I'll let you write it, if you've got that existing mochitest that you can just tweak a bit?  Or, if you point me at where to find that existing mochitest, I'd still be happy to write one.
Ooops wrong bug number. You want to look at bug 463934
Created attachment 351277 [details] [diff] [review]
mochitest (as patch)

Here's a mochitest.  I've confirmed that it fails pre-patch but passes post-patch. 

The failures pre-patch are:
  not ok - bounds top got 48, expected 23
  not ok - bounds bottom got 53, expected 77
  not ok - bounds height got 5, expected 54
which is consistent with the behavior of testcase 2. (The invalidated region there seems to be missing its top & bottom chunks.)

Robert (Longson), could you take a quick look and confirm that this is what you had in mind?
Attachment #351277 - Flags: review?(longsonr)
(In reply to comment #18)
> We measure without the scale applied. Otherwise if you had a rotation rather
> than a scale there would be no way to get the correct extents. That's what bug
> 460946 fixed.

This patch doesn't actually fix the bug, it just gets lucky and avoids triggering it. The ellipse code constructs the circle from four splines instead of the two splines that the arc code uses. Using four splines instead of two acts as more accurate approximation of a circle which causes more accurate extents to be computed.

I believe the correct way to solve this is to apply the transformation matrix before generating the path and then remove the transformation matrix before computing the extents so that the returned extents are in device space.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 351314 [details]
testcase 3: avoid using circle and use curves directly.

Here's a test case that reproduces the bug using a curved path instead of a circle.
(In reply to comment #31)
> I believe the correct way to solve this is to apply the transformation matrix
> before generating the path and then remove the transformation matrix before
> computing the extents so that the returned extents are in device space.

That doesn't work for stroke extents because cairo doesn't fix the stroke width until the extents are computed.
Good point. I don't see a good solution other than adding cairo_(path|stroke)_device_extents() api to cairo. As workarounds we could increase the tolerance, hoping that will help or we could extract the scale from the transformation matrix and compute scaled extents.
I think we should backout the patch (1d84189da181) then.
Comment on attachment 351277 [details] [diff] [review]
mochitest (as patch)

This is exactly what I had in mind.  Looks like my solution is flawed so we shouldn't land this though :-(
Attachment #351277 - Flags: review?(longsonr)
Attachment #351277 - Flags: review+
Whiteboard: [needs 191 landing] → [c-n: what checkin/backout and where ?] [needs 191 landing]
Comment on attachment 349263 [details] [diff] [review]
patch

Backed out.
Attachment #349263 - Attachment is obsolete: true
Assignee: longsonr → nobody
Keywords: checkin-needed
Whiteboard: [c-n: what checkin/backout and where ?] [needs 191 landing]
Progress on this?  Who is going to take this one?
Needs someone to implement the cairo api change per comment 34 I guess.
I'll try to get the cairo api change in soon...
You have to have reasonably unusual SVG to see this though. I.e. you have to define something really small, draw it scaled up and then change its location.
Assigning to Jeff temporarily to get the cairo patch done.
Assignee: nobody → jmuizelaar
Jeff offers this code as the basis for estimate the stroke extents based on the path extents:

_cairo_stroke_style_max_distance_from_path (const cairo_stroke_style_t *style,
                                            const cairo_matrix_t *ctm,
                                            double *dx, double *dy)
{
    double style_expansion = 0.5;
    
    if (style->line_cap == CAIRO_LINE_CAP_SQUARE)
        style_expansion = M_SQRT1_2;
    
    if (style->line_join == CAIRO_LINE_JOIN_MITER &&
        style_expansion < style->miter_limit)
    {
        style_expansion = style->miter_limit;
    }

    style_expansion *= style->line_width;

    *dx = style_expansion * (fabs (ctm->xx) + fabs (ctm->xy));
    *dy = style_expansion * (fabs (ctm->yy) + fabs (ctm->yx));
}

We should be able to use this to work around the cairo API design deadlock.
Okay, that looks okay as a workaround for now. I'll implement that in SVG land for 1.9.1, but it would be good to have cairo API for getting the tight extents at some point if that operation isn't too expensive. Do we need a separate bug for that?
Assignee: jmuizelaar → jwatt
(In reply to comment #44)
> Okay, that looks okay as a workaround for now. I'll implement that in SVG land
> for 1.9.1, but it would be good to have cairo API for getting the tight extents
> at some point if that operation isn't too expensive. Do we need a separate bug
> for that?

Yeah, I think so. I've added bug 478152 to track it.
Just for my own comprehension, here's what happened.

Before bug 460946, we would compute the path's covered area by a) setting up the CTM b) generating the path c) getting cairo's user-space stroke extents and d) converting those to device space. This caused bug 460946 because in step c), cairo would compute the device-space stroke extents, convert them to the user-space bounding box of that device-space rectangle (potentially enlarging it a lot if there's rotation), and then in step d) we'd enlarge the box again when we convert it back to device space and take the bounding-box again.

So we fixed bug 460946 by a) setting the cairo matrix to identity, ignoring the SVG CTM b) generating the path c) getting cairo's user-space stroke extents d) converting those to SVG device space by applying the SVG CTM and taking the bounding box. Unfortunately that triggered this bug, because the SVG user-space coordinates we're using are out of the range that cairo can really deal with as device coordinates.

So what we have to do is basically revert bug 460946 and fix it differently. Ideally we would a) set up the CTM b) generate the path c) set the identity matrix d) get cairo's user-space stroke extents (which are now device space). But step d) breaks because the stroke width is not locked, and changing that behaviour or adding new API for it is not popular in some circles. We could also replace steps c) and d) with "c) get cairo's device-space stroke extents via a new cairo API" but apparently that new API is not popular in other circles.

So until one of those solutions becomes available, we should revert bug 460946 and fix it by a) setting up the CTM b) generating the path c) set the identity matrix d) get the fill extents and e) expanding the fill extents to a conservative estimate of the stroke extents, using the code in comment #43.
That matches my understanding, thanks for summarizing.
Created attachment 362134 [details] [diff] [review]
patch

So here's what I have ATM. I've not quite fully got my head around the consequences of the change to nsSVGGlyphFrame yet, especially what I should be doing with the SetLineWidth(), if anything.
Hmm, I've at least forgotten the context.IdentityMatrix() in nsSVGGlyphFrame.
I wrote some mochitests in bug 463934 but they never landed. Although they will fail on non-windows they should be pretty close. If your results are way out you've done something wrong.

+  static gfxRect PathExtentsToMaxStrokeExtents(gfxRect aPathExtents,
+                                               nsSVGGeometryFrame* aElement);

const & for aPathExtents, also why is the second argument called aElement if it is a frame? aFrame or aGeometryFrame would be less confusing.

+   * exploit the fact that cairo does have API for getting the tight device

s/does have API/does have an API/
Created attachment 369234 [details] [diff] [review]
updated wip

Thanks for the tests Robert, they showed up some issues with this approach (and a bug I've still to figure out).

The problem with reusing the logic from _cairo_stroke_style_max_distance_from_path is that we compute stroke bounds that are substantially bigger than the actual stroke bounds. Since the default value for the 'stroke-linejoin' property is 'miter', and the default value for 'stroke-miterlimit' is '4', we end up with:

  double dx = stroke-miterlimit * stroke-width * (fabs(a) + fabs(c));
  double dy = stroke-miterlimit * stroke-width * (fabs(d) + fabs(b));

Let's take the case of a shape that has a modest stroke of say 10% the widest dimension of the shape. Specifically, in the case of a 100 x 100 rect with a 10px stroke, the actual stroke bounds of the rect are 110 x 110, but we would compute it as 180 x 180. That's quite a lot larger.

The impact is not only that we'll invalidate much larger areas than we currently invalidate, but also that getBoundingClientRect() returns almost uselessly large "bounds".
Attachment #362134 - Attachment is obsolete: true
I should have said that I was assuming that the identity matrix for the CTM, so (fabs(a) + fabs(c)) == (fabs(d) + fabs(b)) == 1. Things don't get better when you start using a non-identity transform though.
Created attachment 369246 [details] [diff] [review]
patch using Jeff's suggestion

Okay, I figured out the bug. The problem was that the bounds for text was being calculated as if the transform was being ignored. That's because it was. CharacterIterator::SetupForDirectTextRun calls aContext->SetMatrix(mInitialMatrix), and mInitialMatrix is the identity matrix unless we set it. The stack looks like this:

  gfxContext::SetMatrix
  CharacterIterator::SetupForDirectTextRun
  CharacterIterator::SetupForDirectTextRunMetrics
  nsSVGGlyphFrame::AddBoundingBoxesToPath
  nsSVGGlyphFrame::UpdateCoveredRegion

I've added iter.SetInitialMatrix(tmpCtx) in nsSVGGlyphFrame::UpdateCoveredRegion to fix this.
Attachment #369246 - Flags: review?(roc)
In some cases the stroke can be so thin cairo returns no bounds as it would not draw the stroke even though one exists. In that case we have to use the fill extents. You need a testcase with a really thin stroke, something reduced from bug 437704 perhaps.

That's why the current logic does a union of fill/stroke extents.
Jeff, is there no way we could hack our version of cairo for now to expose some temporary API to get the real stroke extents until cairo folks agree how to do it right. I don't actually think that this approach is acceptable, given the oversized bounds problem I explained in comment 51.
Robert: ah, I see. I assumed it was to cover the dashing case explained inline in my comments on the patch. I wish people would add more comments documenting why non-obvious things are the way they are. ;-) Thanks for pointing that out.
Attachment #369246 - Attachment is patch: true
Attachment #369246 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Updated

9 years ago
Attachment #369234 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Summary: SVG Repaint issue using JS animation → Small objects that are scaled up don't get invalidated correctly
The problem with adding cairo API locally without also having it upstream is that it means --enable-system-cairo won't work anymore.

I didn't think about how bad it would be to assume the worst about the miter limit... But I think we should take this patch to fix invalidation bugs and then file another bug about getting the correct stroke extents to fix getBoundingClientRect.
Attachment #369246 - Flags: review?(roc) → review+
Comment on attachment 369246 [details] [diff] [review]
patch using Jeff's suggestion

+nsSVGUtils::PathExtentsToMaxStrokeExtents(gfxRect aExtents,

const gfxRect&

+  if (aExtents.Width() == 0 && aExtents.Height() == 0) {

aExtents.IsEmpty()

   }
-
+  
   // Add in markers

Don't add trailing whitespace.

We can have an invalidation reftest here.
I pushed http://hg.mozilla.org/mozilla-central/rev/6d229acfb6d5

(In reply to comment #58)
> +  if (aExtents.Width() == 0 && aExtents.Height() == 0) {
> 
> aExtents.IsEmpty()

It does not have this change. The problem is that <line> elements that are horizontal or vertical have zero height or width respectively for their path extents. Thus extents.IsEmpty() returns true, and if we used that they'd get zero stroke extents too, and as a result would not paint. I did try messing around with code to check for that case, but in the end I think it's just simpler to leave it as it is.
(In reply to comment #57)
> I think we should take this patch to fix invalidation bugs and
> then file another bug about getting the correct stroke extents to fix
> getBoundingClientRect.

A bug other than bug 478152?
Comment on attachment 351277 [details] [diff] [review]
mochitest (as patch)

Jonathan, you should be able to land this too if you think it's worth it.
Also I'm pretty sure bug 478152 covers Roc's suggestion.
The problem with the Mochitests is that the values are actually bad (much too big, due to the problem explained in comment 51), but since they're equally bad in both cases, the test passes. Really we should be able to compare against hard values, rather than using the reftest strategy (comparing one object to another) in mochitests. I think that once bug 478152 is fixed, we should do exactly that.
(Assignee)

Updated

9 years ago
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs 191 landing]
I filed bug 486114 for mochitests.
This appears to have had a nice win on Tsvg at least on the mac.

http://graphs.mozilla.org/#show=794438,794456,794447&sel=1238329253,1238674853
Created attachment 370204 [details] [diff] [review]
adds M_SQRT_1_2 to the shunt

wince was busted by this push, because its math.h doesn't have M_SQRT_1_2.  Adding it to the shunt fixes that.
Attachment #370204 - Flags: review?(doug.turner)
Comment on attachment 370204 [details] [diff] [review]
adds M_SQRT_1_2 to the shunt

do you want to add a comment on what that value is? :-) 

something like:

/* 1 / sqrt(2) */
Attachment #370204 - Flags: review?(doug.turner) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/7552e5c484bc
And a win on linux:
http://graphs.mozilla.org/#show=395143,395148,395172&sel=1238334226,1238679826
(In reply to comment #69)
> http://graphs.mozilla.org/#show=395143,395148,395172&sel=1238334226,1238679826

That's worrying as this should have increased the size of the covered regions we calculate and therefore reduced performance at the expense of correctness.
s/correctness/ensuring the regions are sufficiently sized not to get artifacts/
(In reply to comment #65)
> This appears to have had a nice win on Tsvg at least on the mac.

That's because we're kinda cheating by no longer asking cairo for stroke bounds. In the long run we're going to need to do that, so the perf improvement will only be temporary.

(In reply to comment #70)
> That's worrying as this should have increased the size of the covered regions
> we calculate and therefore reduced performance at the expense of correctness.

The problem is that the calculations to get the covered region (asking cairo to get fill and stroke extents) are not cheap. :-(
BTW, to be clear, the tests under Tsvg do not test invalidation. Computing covered regions is up front overhead we accept specifically to save cycles when doing invalidation. In pure invalidation tests I'm pretty confident we'd find that the change introduced to fix this bug would actually result in a perf regression.
It's perfectly OK to compute a loose estimate of the bounds for invalidation if that's an overall performance win. We should test it, but it's quite plausible to me that it would be a win even on invalidation-heavy pages. The overestimate of the stroke bounds is proportional to the stroke width, which I suspect that most content uses stroke widths that are reasonably small compared to the size of the path being stroked (or else they're not using mitred joins).
(In reply to comment #66)
> wince was busted by this push, because its math.h doesn't have M_SQRT_1_2. 

VC7.1 doesn't have it by default, but you can enable it with _USE_MATH_DEFINES
Created attachment 370591 [details] [diff] [review]
VC7.1 bustage fix
Attachment #370591 - Flags: superreview?(roc)
Attachment #370591 - Flags: review?(roc)
Attachment #370591 - Flags: superreview?(roc)
Attachment #370591 - Flags: superreview+
Attachment #370591 - Flags: review?(roc)
Attachment #370591 - Flags: review+
(In reply to comment #76)
> Created an attachment (id=370591) [details]
> VC7.1 bustage fix

That line is in nsSVGUtils.h. Do we just need to move the nsSVGUtils.h #include in nsSVGUtils.cpp up to the top?
Created attachment 370600 [details] [diff] [review]
Better VC7.1 bustage fix

Thanks for pointing that out!
Attachment #370591 - Attachment is obsolete: true
Attachment #370600 - Flags: superreview?(roc)
Attachment #370600 - Flags: review?(longsonr)
(In reply to comment #78)
> Created an attachment (id=370600) [details]
> Better VC7.1 bustage fix
> 

I think that's the wrong attachment. It's the same as the last one. 

Also you don't need an sr for something this simple (bug 485544 comment 4).
Created attachment 370602 [details] [diff] [review]
Correct VC7.1 bustage fix

Oops, I mistyped the diff command, and created a new file by mistake...
Attachment #370600 - Attachment is obsolete: true
Attachment #370602 - Flags: review?(longsonr)
Attachment #370600 - Flags: superreview?(roc)
Attachment #370600 - Flags: review?(longsonr)
Comment on attachment 370602 [details] [diff] [review]
Correct VC7.1 bustage fix

Maybe change the comment in nsSVGUtils.cpp to
// include nsSVGUtils.h early to ensure that the definition of M_SQRT1_2 is picked up.

r=longsonr whether you update the comment or not.
Attachment #370602 - Flags: review?(longsonr) → review+
(In reply to comment #79)
> Also you don't need an sr for something this simple (bug 485544 comment 4).

That's from http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/ff56bb7ba88dab52/b27d9b0565c382c3 rather than something I decided BTW. ;-)
Comment on attachment 370602 [details] [diff] [review]
Correct VC7.1 bustage fix

I pushed this (with a slightly different comment) as changeset 212f7ae2a298 to mozilla-central claiming sr=roc since he had plussed the previous iteration.
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5fd4fe17ca1c (including WinCE and VC7.1 fixes).
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
Blocks: 469632
Depends on: 510177
You need to log in before you can comment on or make changes to this bug.