[Azure] Cairo path transforms broken

RESOLVED FIXED in Firefox 17

Status

()

Core
Canvas: 2D
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: kentuckyfriedtakahe, Assigned: kentuckyfriedtakahe)

Tracking

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

Firefox Tracking Flags

(firefox17 fixed, firefox18 fixed)

Details

(URL)

Attachments

(4 attachments, 10 obsolete attachments)

373.75 KB, image/png
Details
1.17 KB, patch
Joe Drew (not getting mail)
: review+
Details | Diff | Splinter Review
3.54 KB, patch
Joe Drew (not getting mail)
: review+
Details | Diff | Splinter Review
22.88 KB, patch
Details | Diff | Splinter Review
Paths don't draw correctly using Azure/Cairo if they need to move between user space and device space. This shows up on asteroidbench test 4 as a bunch of unwanted lines across the canvas when drawing enemy bullets.
Blocks: 781371
Created attachment 654120 [details] [diff] [review]
Part 1.5: Fix cairo path transforms
Attachment #654120 - Flags: review?(joe)
Created attachment 654512 [details] [diff] [review]
Part2: Fixed some test failures and did more clean up

It would probably make sense to just combine these patches.
Attachment #654512 - Flags: review?(joe)
Created attachment 654513 [details] [diff] [review]
Part 1: reftest to make sure the problem stays fixed
Attachment #654513 - Flags: review?(joe)
Created attachment 654514 [details] [diff] [review]
Part 2: Fixed some test failures and did more clean up v2
Attachment #654512 - Attachment is obsolete: true
Attachment #654512 - Flags: review?(joe)
Attachment #654514 - Flags: review?(joe)
Created attachment 654515 [details] [diff] [review]
Part 3: RAII matrix helper
Attachment #654515 - Flags: review?(joe)
Attachment #654514 - Attachment description: 654512: Part2: Fixed some test failures and did more clean up v2 → Part2: Fixed some test failures and did more clean up v2
Attachment #654513 - Attachment description: reftest to make sure the problem stays fixed → Part 1: reftest to make sure the problem stays fixed
Attachment #654515 - Attachment description: RAII matrix helper → Part 3: RAII matrix helper
Attachment #654120 - Attachment description: Fix cairo path transforms → Part 1.5: Fix cairo path transforms
Attachment #654514 - Attachment description: Part2: Fixed some test failures and did more clean up v2 → Part 2: Fixed some test failures and did more clean up v2
Created attachment 654845 [details] [diff] [review]
Part 4: Added reftest and removed todo v2
Attachment #654513 - Attachment is obsolete: true
Attachment #654513 - Flags: review?(joe)
Attachment #654845 - Flags: review?(joe)
Created attachment 655458 [details]
Ugly lines on the left of the canvas
Comment on attachment 654845 [details] [diff] [review]
Part 4: Added reftest and removed todo v2

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

This should be part n+1 because it depends on the fixes in the previous n patches.

::: layout/reftests/canvas/784573-1.html
@@ +1,1 @@
> +<!DOCTYPE html>

Can you add a comment saying what this is testing (that is, device space vs user space paths). Also, <!-- Any copyright is dedicated to the public domain. -->
Attachment #654845 - Flags: review?(joe) → review+
Comment on attachment 654120 [details] [diff] [review]
Part 1.5: Fix cairo path transforms

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

Overall, looks OK, but I need more explanation for why things are correct before I r+.

::: gfx/2d/PathCairo.cpp
@@ +25,5 @@
> +  cairo_new_path(mContext);
> +
> +  if (mDrawTarget) {
> +    mDrawTarget->SetPathObserver(this);
> +  }

How do we get around needing to have a separate context from the draw target, as below?

@@ +40,5 @@
> + , mFillRule(aPathContext->mFillRule)
> +{
> +  Matrix inverse = aTransform;
> +  inverse.Invert();
> +  mTransform = aPathContext->mTransform * inverse;

Can you please add a comment explaining why you're doing this?

@@ +48,3 @@
>  
>    // If we don't have an identity transformation, we need to have a separate
>    // context from the draw target, because we can't set a transformation on its

Shouldn't we always have to duplicate the context and path here? IIRC CairoPathContexts don't know how to share contexts between each other.

@@ +98,5 @@
>    cairo_fill_rule_t rule = cairo_get_fill_rule(mContext);
>  
>    // Duplicate the context.
>    cairo_surface_t* surf = cairo_get_target(mContext);
> +  cairo_matrix_t matrix; 

Extra space at the end of this line.

@@ +104,3 @@
>    cairo_destroy(mContext);
>    mContext = cairo_create(surf);
> +  cairo_set_matrix(mContext, &matrix);  

Also here.

@@ +263,5 @@
>  
>  TemporaryRef<Path>
>  PathBuilderCairo::Finish()
>  {
> +  RefPtr<PathCairo> path = new PathCairo(mPathContext);

We should really file a follow-up bug for the fact that this will duplicate the context unnecessarily.

::: gfx/2d/PathCairo.h
@@ +37,5 @@
>  class CairoPathContext : public RefCounted<CairoPathContext>
>  {
>  public:
>    // Construct a CairoPathContext and set it to be the path observer of
> +  // aDrawTarget with an empty path.

aDrawTarget. Also, modify its path in-place to be empty.

@@ +42,4 @@
>    CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget,
> +                   FillRule aFillRule, const Matrix& aTransform);
> +
> +  // Create a CairoPathContext which is a cope of a previous context with

"copy"

@@ +42,5 @@
>    CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget,
> +                   FillRule aFillRule, const Matrix& aTransform);
> +
> +  // Create a CairoPathContext which is a cope of a previous context with
> +  // applying a new transform.

"an optional transform"?

@@ +85,5 @@
>    operator cairo_t* () const { return mContext; }
>  
>  private: // methods
>    CairoPathContext(const CairoPathContext&) MOZ_DELETE;
> +  bool ClassInvariant();

this is never used
Attachment #654120 - Flags: review?(joe) → review-
Comment on attachment 654514 [details] [diff] [review]
Part 2: Fixed some test failures and did more clean up v2

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

This patch looks like it would be improved by folding it with patch 1.5.

::: gfx/2d/PathCairo.cpp
@@ +23,5 @@
>    cairo_reference(mContext);
>    cairo_set_fill_rule(mContext, GfxFillRuleToCairoFillRule(mFillRule));
> +
> +  aDrawTarget->SetPathObserver(this);
> +  mDrawTarget = aDrawTarget;

It looks like you do this dance on purpose. Can you explain why in a comment?

@@ +250,5 @@
> +  cairo_get_matrix(*mPathContext, &saveMatrix);
> +
> +  cairo_matrix_t matrix;
> +  GfxMatrixToCairoMatrix(mPathContext->GetTransform(), matrix);
> +  cairo_set_matrix(*mPathContext, &matrix);

When do these get out of sync? Shouldn't this always be the case?

::: gfx/2d/PathCairo.h
@@ +43,2 @@
>  
> +  // Transform an existing path

These comments seem worse than the previous ones.

@@ +60,5 @@
>    // our context.
>    void ForgetDrawTarget();
>  
>    // Create a duplicate context, and copy this path to that context. Optionally,
>    // the new context can be transformed.

Update this comment.
Attachment #654514 - Flags: review?(joe) → review-
Comment on attachment 654515 [details] [diff] [review]
Part 3: RAII matrix helper

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

::: gfx/2d/HelpersCairo.h
@@ +206,5 @@
>  
>    return CAIRO_FILL_RULE_WINDING;
>  }
>  
> +class CairoTempMatrix {

Can you document this class? Also put { open braces on their own line for the class and functions.
Attachment #654515 - Flags: review?(joe) → review+
Created attachment 655515 [details] [diff] [review]
Part 0: Fix cairo path transforms v2
Attachment #654120 - Attachment is obsolete: true
Attachment #654514 - Attachment is obsolete: true
Attachment #655515 - Flags: review?(joe)
Created attachment 655524 [details] [diff] [review]
Part 2: RAII matrix helper v2 (rebased)
Attachment #654515 - Attachment is obsolete: true
http://tbpl.mozilla.org/?tree=Try&rev=f4c7e27c718e
Comment on attachment 655515 [details] [diff] [review]
Part 0: Fix cairo path transforms v2

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

Some unanswered questions (some left over from previous reviews) lead me to still need to see a follow-up before r+.

::: gfx/2d/DrawTargetCairo.h
@@ +168,5 @@
>    cairo_t* mContext;
>    cairo_surface_t* mSurface;
>    IntSize mSize;
>    std::vector<SourceSurfaceCairo*> mSnapshots;
> +  mutable CairoPathContext* mPathObserver;

Ownership rules deserve a comment here or on SetPathObserver. Seeing a bare pointer makes my tummy hurt.

::: gfx/2d/PathCairo.cpp
@@ +22,5 @@
>  {
>    cairo_reference(mContext);
>    cairo_set_fill_rule(mContext, GfxFillRuleToCairoFillRule(mFillRule));
>  
> +  aDrawTarget->SetPathObserver(this);

Note: You didn't answer my earlier question, "How do we get around needing to have a separate context from the draw target, as below?", but I think I have figured it out: the non-copy CairoPathContext constructor is only ever called when creating a new path, correct?

@@ +35,5 @@
> + , mFillRule(aPathContext->mFillRule)
> +{
> +  cairo_reference(mContext);
> +
> +  // Canvas is telling us what transform we would apply from device space to

Don't talk about canvas here - it is one of the users of the DrawTarget API, but not the only one.

@@ +75,5 @@
>    cairo_path_destroy(path);
> +
> +  // Transform the context.
> +  GfxMatrixToCairoMatrix(mTransform, matrix);
> +  cairo_set_matrix(mContext, &matrix);

You set the cairo context's matrix to mContext's old matrix, and then set it to mTransform. Why? When do those two get out of sync?

@@ +86,5 @@
> +{
> +  if (mDrawTarget) {
> +    // null the draw target so that we don't try to copy the path when we get
> +    // a PathWillChange notification back.
> +    DrawTargetCairo* drawTarget = mDrawTarget;

Seems like most of this can just be elided, with some comments explaining it and an unconditional mDrawTarget = nullptr;

@@ +250,5 @@
>    inverse.Invert();
>    Point transformed = inverse * aPoint;
>  
> +  // ContainsPoint can be called after the DrawTarget has been notified about
> +  // a change in transform but before the path has been notified by Canvas. In

s/Canvas/DrawTarget/ perhaps? References to canvas don't really make sense in backend code. Perhaps s/Canvas/user code/?

@@ +300,5 @@
>  
>  void
>  PathCairo::CopyPathTo(cairo_t* aContext, DrawTargetCairo* aDrawTarget)
>  {
> +  mPathContext->CopyPathTo(aContext);

Why do you do this unconditionally? Was the previous optimization incorrect? And, if mPathContext->GetContext() == aContext, isn't this going to do useless work?

::: gfx/2d/PathCairo.h
@@ +37,5 @@
>  class CairoPathContext : public RefCounted<CairoPathContext>
>  {
>  public:
> +  // Construct a new empty CairoPathContext that uses the given draw target and
> +  // it's cairo context. Using the existing context  may save having to copy the

its

@@ +62,5 @@
>    // our context.
>    void ForgetDrawTarget();
>  
>    // Create a duplicate context, and copy this path to that context. Optionally,
>    // the new context can be transformed.

Please fix this comment.

@@ +98,5 @@
>  
>    // This constructor, called with a CairoPathContext*, implicitly takes
>    // ownership of the path, and therefore makes aPathContext copy out its path
>    // regardless of whether it has a pointer to a DrawTargetCairo.
>    // The path currently set on aPathContext is not changed.

s/aPathContext/aContext/g
Attachment #655515 - Flags: review?(joe) → review-
Created attachment 655817 [details] [diff] [review]
Part 3: Fix cairo path transforms v3

Comment 17

5 years ago
Try run for fa656a8c5699 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fa656a8c5699
Results (out of 17 total builds):
    exception: 17
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-fa656a8c5699
Status: NEW → ASSIGNED
Attachment #655515 - Attachment is obsolete: true
Attachment #655817 - Flags: review?(joe)
Created attachment 655888 [details] [diff] [review]
Part 2: RAII matrix helper v3
Attachment #655524 - Attachment is obsolete: true
Attachment #655888 - Flags: review?(joe)
Attachment #654845 - Attachment description: Part 1: Added reftest and removed todo v2 → Part 4: Added reftest and removed todo v2
(In reply to Joe Drew (:JOEDREW!) from comment #15)
> @@ +300,5 @@
> >  
> >  void
> >  PathCairo::CopyPathTo(cairo_t* aContext, DrawTargetCairo* aDrawTarget)
> >  {
> > +  mPathContext->CopyPathTo(aContext);
> 
> Why do you do this unconditionally? Was the previous optimization incorrect?
> And, if mPathContext->GetContext() == aContext, isn't this going to do
> useless work?

CairoPathContext::CopyPathTo() does this check anyway so it's duplicated. The previous code was deciding whether to subscribe to the DrawTarget more than to skip the call. Subscribing to the DrawTarget is a coin toss as to whether it'll save time or create the need for another copy.

Comment 20

5 years ago
Try run for 36ddd2e5b9b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=36ddd2e5b9b6
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-36ddd2e5b9b6

Comment 21

5 years ago
Try run for aace50d4c079 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=aace50d4c079
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-aace50d4c079
Created attachment 656640 [details] [diff] [review]
Part 4: Added reftest and removed todo v3

Fuzzed reftest on D2D
Attachment #654845 - Attachment is obsolete: true
Attachment #656640 - Flags: review?(joe)
Attachment #655888 - Flags: review?(joe) → review+
Comment on attachment 655817 [details] [diff] [review]
Part 3: Fix cairo path transforms v3

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

::: gfx/2d/DrawTargetCairo.cpp
@@ -924,5 @@
> -  // We're about to logically change our transformation. Our current path will
> -  // need to change, because Cairo stores paths in device space.
> -  if (mPathObserver) {
> -    mPathObserver->MatrixWillChange(aTransform);
> -  }

As discussed on IRC, we can't remove this, because it'll break a perfectly reasonable use case:

[4:39 PM] <joe> PathBuilder* pb = dt->CreatePathBuilder()
[4:39 PM] <joe> pb->AddPoints() // or whatever
[4:39 PM] <joe> dt->SetMatrix(something)
[4:39 PM] <joe> pb->AddPoints()
[4:39 PM] <joe> those points will be transformed by the new matrix

Instead, maybe only copy out the path when the path is changed after the matrix is changed?
Attachment #655817 - Flags: review?(joe) → review-
Attachment #656640 - Flags: review?(joe) → review+
Created attachment 657157 [details] [diff] [review]
Part 3: Fix cairo path transforms v4
Attachment #655817 - Attachment is obsolete: true
Attachment #657157 - Flags: review?(joe)

Comment 25

5 years ago
Try run for 2ff039275a33 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2ff039275a33
Results (out of 24 total builds):
    success: 24
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-2ff039275a33
Comment on attachment 657157 [details] [diff] [review]
Part 3: Fix cairo path transforms v4

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

OK! Looks good with some small fixes. Hooray!

::: gfx/2d/PathCairo.cpp
@@ +61,5 @@
>    mContext = cairo_create(surf);
>  
> +  // Set the matrix to match the source context so that the path is copied in
> +  // device space.
> +  cairo_set_matrix(mContext, &matrix);

Ooh, better use a CairoTempMatrix here, so we're reset to identity afterwards.

@@ +88,5 @@
>    if (mDrawTarget) {
>      // The context we point to is going to change from under us. To continue
>      // using this path, we need to copy it to a new context.
>      DuplicateContextAndPath();
> +    mDrawTarget = nullptr;

Better to leave this as a call to ForgetDrawTarget().

@@ +100,3 @@
>      // cairo_copy_path gives us a user-space copy of the path, so we don't have
>      // to worry about transformations here.
> +    CairoTempMatrix tempMatrix(mContext, aTransform);

This line contradicts the comment above - better remove the comment :)

@@ +217,5 @@
> +PathBuilderCairo::PrepareForWrite()
> +{
> +  if (mPathContext->refCount() != 1) {
> +    mPathContext = new CairoPathContext(*mPathContext);
> +  }

Better add a comment saying that only PathBuilder and PathCairo maintain references to CairoPathContext, not DrawTarget, so this will ensure that we're duplicating if we're sharing a context.

::: gfx/2d/PathCairo.h
@@ +42,5 @@
> +  // path later.
> +  CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget);
> +
> +  // Copy the path and supply a new fill rule.
> +  CairoPathContext(CairoPathContext& aPathContext);

This does not seem to supply a new fill rule.

@@ +86,4 @@
>    PathBuilderCairo(cairo_t* aCtx, DrawTargetCairo* aDrawTarget, FillRule aFillRule);
>  
> +  // Creates a path builder out of an existing CairoPathContext with a new fill
> +  // rule and transform. The fill 

Unfinished sentence in this comment.
Attachment #657157 - Flags: review?(joe) → review+
(In reply to Joe Drew (:JOEDREW! \o/) from comment #26)
> ::: gfx/2d/PathCairo.cpp
> @@ +61,5 @@
> >    mContext = cairo_create(surf);
> >  
> > +  // Set the matrix to match the source context so that the path is copied in
> > +  // device space.
> > +  cairo_set_matrix(mContext, &matrix);
> 
> Ooh, better use a CairoTempMatrix here, so we're reset to identity
> afterwards.

We swap out the matrix every time so it doesn't matter what we set it to. We could set it to identity but it would serve no purpose.
Created attachment 657715 [details] [diff] [review]
Part 3: Fix cairo path transforms v5

Made changes as requested.
Attachment #657157 - Attachment is obsolete: true

Comment 29

5 years ago
Try run for 51a9cd52811b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=51a9cd52811b
Results (out of 16 total builds):
    exception: 15
    success: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-51a9cd52811b

Comment 30

5 years ago
Try run for 73be938dae67 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=73be938dae67
Results (out of 110 total builds):
    exception: 63
    success: 42
    warnings: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-73be938dae67

Comment 31

5 years ago
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 194 total builds):
    exception: 1
    success: 168
    warnings: 21
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882

Comment 32

5 years ago
Try run for 23bf9f4a005b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=23bf9f4a005b
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-23bf9f4a005b

Comment 33

5 years ago
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 196 total builds):
    exception: 1
    success: 169
    warnings: 21
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882

Comment 34

5 years ago
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 197 total builds):
    exception: 1
    success: 170
    warnings: 21
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882

Comment 35

5 years ago
Try run for 0fcdb2834678 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Results (out of 47 total builds):
    success: 45
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-0fcdb2834678

Comment 36

5 years ago
Try run for 0fcdb2834678 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Results (out of 49 total builds):
    success: 47
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-0fcdb2834678
Here are the results for right patch set.

https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/92605c1168b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/2879ae03fa01
https://hg.mozilla.org/integration/mozilla-inbound/rev/eeda05ede83f
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/92605c1168b2
https://hg.mozilla.org/mozilla-central/rev/2879ae03fa01
https://hg.mozilla.org/mozilla-central/rev/eeda05ede83f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

5 years ago
Blocks: 803658
Bug 803658 suggests we need this fix on beta? If so, please request beta approval.
Comment on attachment 655888 [details] [diff] [review]
Part 2: RAII matrix helper v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 773460 enable azure canvas
User impact if declined: broken canvas transforms
Testing completed (on m-c, etc.): in m-c for several weeks before aurora uplift
Risk to taking this patch (and alternatives if risky): May cause other canvas path breakages or may not fix everything. Low risk on content because content doesn't use Azure Cairo backend.
String or UUID changes made by this patch: none
Attachment #655888 - Flags: approval-mozilla-beta?
Attachment #656640 - Flags: approval-mozilla-beta?
Attachment #657715 - Flags: approval-mozilla-beta?
Attachment #655888 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #656640 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #657715 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/56775cef1804
https://hg.mozilla.org/releases/mozilla-beta/rev/67dda9d8aac3
https://hg.mozilla.org/releases/mozilla-beta/rev/82537ef2f395
status-firefox17: --- → fixed
status-firefox18: --- → fixed
Depends on: 1124549
You need to log in before you can comment on or make changes to this bug.