Seth Fowler [:seth] [:s2h] Assignee
	Description • 11 years ago

After bug 600207, which involved some changes to the matrices used for drawing SVG images, there are some cases in which SVG images fail to tile. I don't yet know specifically why the failure only occurs in some cases and not others.

I first noticed the problem when looking at this mockup of the Australis theme in the current Nightly:

http://people.mozilla.com/~shorlander/files/australis-linux-svg-test/australis-liveDemo-linux.html

Right now the background of the selected tab fails to tile. The same mockup looks fine in the Nightly build before the one which included bug 600207, so I strongly suspect that bug 600207 is the cause.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 1 • 11 years ago

Bug 885919 is related; an appropriate reftest would have caught this bug.

	Daniel Holbert [:dholbert]
	Comment 2 • 11 years ago

(Note that we do still successfully render attachment 761196 [details] , which has a tiled svg image as a css background, FWIW.)

	Daniel Holbert [:dholbert]
	Comment 3 • 11 years ago

Actually, it looks like the CSS for that tab-title is:

.tab.active {
  position: relative;
  background-image: url(../images-linux/tabActiveStart.svg),
                    url(../images-linux/tabActiveMiddle.svg),
                    url(../images-linux/tabActiveEnd.svg),
                    url(../images-linux/tabActiveStart.png),
                    url(../images-linux/tabActiveMiddle.png),
                    url(../images-linux/tabActiveEnd.png);
  background-repeat: no-repeat,
                     no-repeat,
                     no-repeat,
                     no-repeat,
                     no-repeat,
                     no-repeat;
  background-position: left,
                       30px,
                       right,
                       left,
                       30px,
                       right;
  background-size: 30px 31px,
                   calc(100% - 60px) 31px,
                   30px 31px,
                   30px 31px,
                   calc(100% - 60px) 31px,
                   30px 31px;

which looks like it's *not* tiling-related (given the "no-repeat").

Source: http://people.mozilla.com/~shorlander/files/australis-linux-svg-test/css/styles-linux.css

So still a background-image issue, but not necessarily a tiling issue.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 4 • 11 years ago

Yep, looks like it's being stretched, not tiled.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 5 • 11 years ago

Alrighty, so the problem is that we can't use RenderDocument to directly render an SVG document with nonuniform scale factors (that is, where x and y are scaled differently). The solution is to only factor a uniform scale factor out of the transform. I chose to factor out the largest scale factor, so after the transformation we always render larger and then shrink down in a nonuniform fashion as appropriate. This should provide the highest image quality.

This resolves the Australis tab testcase and some other ones. (It does NOT address all known problems with scaling; I'll address the remainder in a separate bug.)

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 6 • 11 years ago

This adds a reftest that tests the ability of SVG images to stretch (which is the clearest English way I can think of describing scaling in a nonuniform fashion). If we had this reftest, we would have caught this bug before it hit m-c; the test fails before Part 1 is applied and passes with it applied. It's based on the Australis tab test case.

	Daniel Holbert [:dholbert]
	Comment 7 • 11 years ago

Comment on attachment 773475 [details] [diff] [review]
(Part 2) - Add reftest for SVG image stretching.

A few nits on the SVG file in the reftest:

>+++ b/layout/reftests/svg/as-image/white-rect.svg
>@@ -0,0 +1,16 @@
>+<?xml version="1.0" encoding="utf-8"?>
>+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">

Drop the <!DOCTYPE line -- it's unnecessary, and our view-source UI actually warns about it (highlights it in red) as a "stray doctype".

>+<svg version="1.1"
>+     xmlns="http://www.w3.org/2000/svg"
>+     xmlns:xlink="http://www.w3.org/1999/xlink"
>+     x="0px"
>+     y="0px"

Nit: drop x/y here. They're meaningless on outermost SVG elements.
 http://www.w3.org/TR/SVG11/struct.html#SVGElementXAttribute

>+     width="6px"
>+     height="30px"
>+     viewBox="0 0 6 30"
>+     xml:space="preserve">

Drop this xml:space thing, too; it adds nothing.

>+  <rect width="6" height="30" style="fill:white; fill-opacity:1;" />

Drop "fill-opacity" -- it defaults to 1, so there's no reason to bother setting it.
 http://www.w3.org/TR/SVG11/painting.html#FillOpacityProperty

	Daniel Holbert [:dholbert]
	Comment 8 • 11 years ago

Comment on attachment 773475 [details] [diff] [review]
(Part 2) - Add reftest for SVG image stretching.

A few more reftest nits:

>+++ b/layout/reftests/svg/as-image/background-stretch-1-ref.html
>@@ -0,0 +1,39 @@
>+<!DOCTYPE html>
>+<head>
>+  <style>
>+    div {
>+      background-color: rgb(100, 100, 0);
>+      color: rgb(51, 51, 51);

This "color" has no effect, right? It controls text-color, but there's no text here.

>+      margin: 0px;
>+      padding: 0px;

These are the initial values of margin/padding; no need to set them explicitly, right?
(The same goes for the testcase)

>+      height: 30px;
>+    }
>+    .container {
>+      width: 100px;
>+    }
>+    .stretch {
>+      width: 40px;
>+      background-color: white;
[...]
>+    body {
>+      background-color: rgb(0, 0, 0);

I'd prefer that we keep this consistent on color names vs. rgb() values.  Names are more human readable, so maybe let's use "black" instead of rgb(0, 0, 0), and maybe "yellow" instead of rgb(100, 100, 0)?
(The same goes for the testcase)

>+++ b/layout/reftests/svg/as-image/reftest.list
>+# Test for stretching background images
>+== background-stretch-1.html background-stretch-1-ref.html

Clarify slightly, to e.g.:
# Test for stretching background images by different amounts in each dimension

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 9 • 11 years ago

(In reply to Seth Fowler [:seth] from comment #5)
> Alrighty, so the problem is that we can't use RenderDocument to directly
> render an SVG document with nonuniform scale factors (that is, where x and y
> are scaled differently).

I knew there was a better way to express this and after posting the patch I realized it. RenderDocument _preserves the aspect ratio_ of the underlying SVG document.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 10 • 11 years ago

(In reply to Daniel Holbert [:dholbert] from comment #8)

All good nits, thanks! I'll get a new version cooked up shortly.

	Daniel Holbert [:dholbert]
	Comment 11 • 11 years ago

(In reply to Seth Fowler [:seth] from comment #9)
> RenderDocument _preserves the aspect ratio_ of the underlying
> SVG document.

Hm... I'm not sure I'm convinced of this. I'd expect it to render the viewport-region that it knows about, to the gfxContext that it's passed.

So what I *suspect* is happening is something like this:
We're probably adjusting the viewport and matrices in parallel, such that the "stretched" viewport region ends up mapping back to "un-stretched" SVG coordinates, on the actual underlying render-target...  Or something like that.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 12 • 11 years ago

(In reply to Daniel Holbert [:dholbert] from comment #11)
> Hm... I'm not sure I'm convinced of this.

After some deep investigation of the other scaling issues I mentioned I agree; that this was happening may just have been the result of another bug. There was a more fundamental issue here - scaling the viewport size is a bad idea and we end up having to reproduce several calculations in VectorImage.cpp that the SVG document should be doing internally. If we just don't scale the viewport, everything becomes a lot simpler!

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 13 • 11 years ago

This new patch _does_ fix all known SVG-as-image scaling issues introduced by bug 600207. We don't scale the viewport at all; instead, we pass an additional matrix to SVGDrawingCallback which we apply just before drawing. Because we're still scaling the texture into which we're drawing, we still get the benefits of bug 600207, but the scaling issues are fixed.

	Daniel Holbert [:dholbert]
	Comment 14 • 11 years ago

Comment on attachment 773697 [details] [diff] [review]
(Part 1) - Make SVG images scale correctly.

Thanks! I think this makes a lot more sense.

One thing I noticed that can be improved:

> // Helper-class: SVGDrawingCallback
> class SVGDrawingCallback : public gfxDrawingCallback {
> public:
>   SVGDrawingCallback(SVGDocumentWrapper* aSVGDocumentWrapper,
>                      const nsIntRect& aViewport,
>+                     const gfxMatrix& aMatrix,
[...]
> private:
>   nsRefPtr<SVGDocumentWrapper> mSVGDocumentWrapper;
>   const nsIntRect mViewport;
>+  const gfxMatrix mMatrix;

"mMatrix" is kind of a generic name, given the number of matrices involved here. :)  This could probably use a comment, EXCEPT: it actually looks like this will always just be a scaling matrix -- so, instead of storing a matrix here, how about we just pass in a gfxSize for the scale factors?  (We actually already have exactly the gfxSize to pass in -- "scale", in VectorImage::Draw.)

>@@ -266,16 +269,17 @@ SVGDrawingCallback::operator()(gfxContex
[...]
>   gfxContextMatrixAutoSaveRestore contextMatrixRestorer(aContext);
>   aContext->Multiply(gfxMatrix(aTransform).Invert());
>+  aContext->Multiply(gfxMatrix(mMatrix).Invert());

Then this can be:
    aContext->Scale(1.0 / mScale.width,
                    1.0 / mScale.height)
right?

	Daniel Holbert [:dholbert]
	Comment 15 • 11 years ago

So I'm also a little uncomfortable with the "drawableSize" vs. "viewportSize" distinction.

It's partly a naming thing -- we have aViewportSize, which we rescale and call drawableSize, which we then pass in as the "aViewportSize" argument to SVGDrawingCallback (and store it as "mViewportSize"), and then we treat it as if it is, actually, the viewport size (even though it's not?)  (That last part is the non-naming part of my confusion about this.)

	Daniel Holbert [:dholbert]
	Comment 16 • 11 years ago

er, I guess that was "mViewport", not "mViewportSize", in the DrawingCallback, but the naming point stands.

>  we treat it as if it is, actually, the viewport size
To clarify, I'm talking about this (in the existing code):
> 278   nsRect svgRect(presContext->DevPixelsToAppUnits(mViewport.x),
> 279                  presContext->DevPixelsToAppUnits(mViewport.y),
> 280                  presContext->DevPixelsToAppUnits(mViewport.width),
> 281                  presContext->DevPixelsToAppUnits(mViewport.height));
[...]
> 288   presShell->RenderDocument(svgRect, renderDocFlags,
> 289                             NS_RGBA(0, 0, 0, 0), // transparent
> 290                             aContext);

The point is to render the whole viewport there -- the same viewport we told the presShell about, up a view levels, via our call to mSVGDocumentWrapper->UpdateViewportBounds(aViewportSize).

If we're using a scaled size there, won't we be rendering something that's the wrong size? (too large or too small)  It may not matter, visibly, if we're rendering something that's large, since we're clipping, but it's still potentially wasteful...

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 17 • 11 years ago

(In reply to Daniel Holbert [:dholbert] from comment #15)
> So I'm also a little uncomfortable with the "drawableSize" vs.
> "viewportSize" distinction.
> 
> It's partly a naming thing -- we have aViewportSize, which we rescale and
> call drawableSize, which we then pass in as the "aViewportSize" argument to
> SVGDrawingCallback (and store it as "mViewportSize"), and then we treat it
> as if it is, actually, the viewport size (even though it's not?)  (That last
> part is the non-naming part of my confusion about this.)

Heh, actually I changed the name to _emphasize_ this distinction. Note that we do not pass drawableSize to SVGDrawingCallback - for SVGDrawingCallback's aViewport parameter, we (essentially) pass in aViewportSize directly. We pass drawableSize to gfxCallbackDrawable because that's the size we're ultimately going to draw at.

Does that make sense?

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 18 • 11 years ago

(In reply to Daniel Holbert [:dholbert] from comment #16)
> To clarify, I'm talking about this (in the existing code):

I think this is actually not happening - we do indeed pass aViewportSize through to SVGDrawingCallback in the way you want. I've been staring at this code for hours so I may be misreading it, but I'm pretty sure that's true. =)

	Daniel Holbert [:dholbert]
	Comment 19 • 11 years ago

Oh, sorry - I misread. I saw drawableTransform being passed to SVGDrawingCallback, and misread it as drawableSize (which I thought was being passed as the viewport size).

So: never mind about that, thanks for clearing that up. :) Re-reading with that in mind...

(Disregard comment 15 and 16; sorry for the confusion)

(In reply to Seth Fowler [:seth] from comment #17)
> Heh, actually I changed the name to _emphasize_ this distinction.

(Yup, that makes sense - I was just concerned that we were losing the rename, one level of abstraction down. But again, I was misreading. :))

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 20 • 11 years ago

I've addressed all the review comments on the original version of part 2. I've also added a couple of new tests, so I'm rerequesting review. Only the "no-viewbox" version fails without the patch in part 1, but I included both just to be thorough, since this stuff didn't have good test coverage up to now.

	Daniel Holbert [:dholbert]
	Comment 21 • 11 years ago

Comment on attachment 773697 [details] [diff] [review]
(Part 1) - Make SVG images scale correctly.

OK, one other optional nit:

>   gfxRect sourceRect = unscaledTransform.Transform(aFill);
[...]
>+  gfxRect imageRect(0, 0, drawableSize.width, drawableSize.height);
>+  gfxRect subimage(aSubimage.x, aSubimage.y,
>+                   aSubimage.width, aSubimage.height);
>+  subimage.ScaleRoundOut(1.0 / scale.width, 1.0 / scale.height);

It's a non-obvious why we're creating a variable 'subimage' here, as a rescaled version of |aSubimage|. If we're scaling it, we probably should give it a more clearly-distinct name.

(Looks like it *really* already is a rescaled version of |aSubimage|, in m-c, going at least as far back as bug 600207's patch, but it's more obviously-confusing now that it's getting its value *directly* from aSubimage. :))

SO: Maybe rename these 3 variables (sourceRect, imageRect, and subimage) to have a "drawable" prefix, to distinguish them (particularly 'subimage') from any imagelib analogs? (These ones are in terms of our drawable's coordinate-space, IIUC, hence the 'drawable' prefix -- to match 'drawableSize')

Aside: That also makes me wonder whether we should be scaling 'aFill' as well... looks like that's the only sizing arg that we pass *directly* through to DrawPixelSnapped, without any scaling...  That's probably a separate issue, though.

	Daniel Holbert [:dholbert]
	Comment 22 • 11 years ago

Comment on attachment 773697 [details] [diff] [review]
(Part 1) - Make SVG images scale correctly.

>Bug 885939 (Part 1) - Make SVG images scale correctly. r=dholbert

This commit message is also a bit vague. (And honestly, probably misleading, since there are very likely still a some other svg image-scaling bugs that we haven't yet fixed :))

How about:
 "For scaled SVG images, resize the gfxDrawable instead of the viewport"
or something like that?

r=me on part 1, with a clearer commit message and comment 14 and comment 21 addressed / responded-to.

	Daniel Holbert [:dholbert]
	Comment 23 • 11 years ago

Comment on attachment 773715 [details] [diff] [review]
(Part 2) - Add reftests for SVG image stretching and scaling.

Nit: Looks like all of the -ref files, along with the testcase background-stretch-1.html, are missing <html></html> wrappers.

Probably worth adding those, for consistency.

>+++ b/layout/reftests/svg/as-image/background-stretch-1-ref.html
>+<body>
>+  <div class="container">
>+    <div class="left-spacer"></div>
>+    <div class="stretch"></div>
>+    <div class="right-spacer"></div>
>+  </div>
>+</body>
>+

This reference case has a straggling newline at the end -- remove it (or populate it with </html>) :)

>+++ b/layout/reftests/svg/as-image/background-stretch-1.html
[...]
>+<body>
>+  <div></div>
>+</body>
>+

Same with this testcase.

r=me with that

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 24 • 11 years ago

Thanks for the reviews!

(In reply to Daniel Holbert [:dholbert] from comment #14)
> so, instead of storing a matrix
> here, how about we just pass in a gfxSize for the scale factors?

That sounds good; I'll make that change.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 25 • 11 years ago

(In reply to Daniel Holbert [:dholbert] from comment #21)
> SO: Maybe rename these 3 variables (sourceRect, imageRect, and subimage) to
> have a "drawable" prefix, to distinguish them (particularly 'subimage') from
> any imagelib analogs?

Sure, I'll make that change. I agree that that seems more consistent, now that I've introduced the 'drawable' concept.

> Aside: That also makes me wonder whether we should be scaling 'aFill' as
> well... looks like that's the only sizing arg that we pass *directly*
> through to DrawPixelSnapped, without any scaling...  That's probably a
> separate issue, though.

I wondered that when I was originally making the patch for bug 600207, but after some thought (and experimentation to verify my reasoning) it became clear that aFill doesn't need to be scaled. The reason is that aFill is the rect that we're drawing _into_, and that doesn't change. All that we're really changing here is the way that we express the other drawing parameters to map the image into that rect. The intention is that the final image will be the same size as it would've been before.

	Daniel Holbert [:dholbert]
	Comment 26 • 11 years ago

Ah, makes sense. Thanks for the explanation!

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 27 • 11 years ago

Addressed review comments.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 28 • 11 years ago

Addressed review comments.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 29 • 11 years ago

All that's left is a try push, but I'm not having much luck pushing to try. Will give it another shot later today.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 30 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=7ca818b56ec0

	Daniel Holbert [:dholbert]
	Comment 32 • 11 years ago

As noted in (duplicate) bug 890009 comment 1, this bug causes an icon in the apple.com header to render at the wrong size, during full-page zoom.

This bug is a regression introduced by bug 600207's patch, which landed in the Firefox 24 timeframe. Once we've fixed this on trunk, we should probably backport the fix to Firefox 24 (currently Aurora) to be sure we don't ship this regression when 24 hits release.

	Daniel Holbert [:dholbert]
	Comment 33 • 11 years ago

So IIUC, this patch scales our viewport to be closer to the size of the background-region that's being drawn.  In this case, that means we scale down from a huge number, and we end up getting 0 instead of something smallish, due to rounding error.  I think.

(In particular, if I put a breakpoint in nsSVGOuterSVGFrame::BuildDisplayList() and print out the frame tree, I get a SVG rect with nonzero width in current mozilla-central, but with zero width with the patch applied.  (using one of the failing testcases))

This is technically a bug (though it may not be one we care about very much, since it's so edge-casey).  I'd suggest filing a followup on this, and then annotate the failing tests as "fails" with a mention of that bug.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 34 • 11 years ago

This patch disables the failing reftests, per discussion between dholbert and me. I have not been able to resolve the issue so far. Since this is a corner case that only occurs when extreme coordinates are involved, while the patch resolves problems that occur frequently on the web, I think it's worth it to go ahead and push this in. I'll create a new bug to try to get this issue solved at some point in the future.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 35 • 11 years ago

I should provide more info to clear up some things in comment 33:

- This patch does not scale the viewport at all. It removes viewport scaling, returning us (in that respect) to the status quo before bug 600207. There definitely is an issue with overflow somewhere, but I strongly suspect it is occurring in the graphics subsystem rather than anywhere to do with the viewport.

- I am not able to reproduce dholbert's report above of the SVG rect getting zero width with this patch applied. Not sure what's going on there, but at this point I think that's a red herring. Really, the thing that has changed here is the transformation matrix on the Thebes context when we call RenderDocument. Something, somewhere, is not handling that well.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 36 • 11 years ago

Update the bug reference to refer to the new bug.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 37 • 11 years ago

Try is not currently accessible but as far as I know those tests were the only issues with this patch. I've gone ahead and pushed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c49f7bbd129
https://hg.mozilla.org/integration/mozilla-inbound/rev/de46c9ce71ef
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2f1cc4b2a41

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 38 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/7c49f7bbd129
https://hg.mozilla.org/mozilla-central/rev/de46c9ce71ef
https://hg.mozilla.org/mozilla-central/rev/c2f1cc4b2a41

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 39 • 11 years ago

Comment on attachment 774862 [details] [diff] [review]
(Part 1) - Scale SVG images using the graphics context matrix instead of the viewport.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 600207
User impact if declined: Rendering issues on popular websites that use SVG images (e.g. apple.com).
Testing completed (on m-c, etc.): The patch has been on m-c for 5 days with no known issues.
Risk to taking this patch (and alternatives if risky): This patch involves substantial changes to the way SVG images are rendered, which does have some risk, but since the current implementation is broken in a way that is likely to have a serious negative impact on user experience, I think the risk is justified.
String or IDL/UUID changes made by this patch: None.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 40 • 11 years ago

Comment on attachment 774864 [details] [diff] [review]
(Part 2) - Add reftests for SVG image stretching and scaling.

[Approval Request Comment]

See above.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 41 • 11 years ago

Comment on attachment 776614 [details] [diff] [review]
(Part 3) - Disable SVG reftests involving extreme viewboxes that fail due to integer overflow.

[Approval Request Comment]

See above.

	Daniel Holbert [:dholbert]
	Comment 42 • 11 years ago

For approval purposes: note also that this bug is a regression, which currently only affects Aurora (since it regressed when current-aurora was on trunk).

If we backport the fix to Aurora, then that will prevent us from ever shipping this regression to release/beta users, which would be great.

	bhavana bajaj [:bajaj]
	Comment 43 • 11 years ago

Comment on attachment 774862 [details] [diff] [review]
(Part 1) - Scale SVG images using the graphics context matrix instead of the viewport.

Given this is a regression and that it impacts high-profile web-sites, looks fine to land.

Can you please recommend applications/websites that may be SVG intensive so QA can help verify those for sure along with the regular Aurora/Beta testing that we'd get.that way we'd be proactive on our side and in a way mitigate the medium risk that was pointed here.

	Ryan VanderMeulen [:RyanVM]
	Comment 44 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/fc9b10335396
https://hg.mozilla.org/releases/mozilla-aurora/rev/44c000c77e9d
https://hg.mozilla.org/releases/mozilla-aurora/rev/5facfc2bf316

	Simona Badau, Desktop QA
	Comment 45 • 11 years ago

Verified as fixed on Firefox 25 beta 4: - the Australis theme mockup and the SVG images from apple.com are properly displayed.

Verified on Windows 7, Ubuntu 12.10 and Mac OS X 10.8:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0
Build ID: 20131001024718

I'm waiting for the additional suggestions requested in Comment 43, before setting the status for Firefox 25 to verified.

	Simona Badau, Desktop QA
	Comment 46 • 11 years ago

Since there are no additional suggestions, setting the status flag for Firefox 25 to verified.