Summary: | ForeignObject content is zoomed two times | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Florin Malita <fmalita> | ||||||||||
Component: | SVG | Assignee: | Florin Malita <fmalita> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | fmalita, hugo, macpherson, menard, webkit.review.bot, zimmermann | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Florin Malita
2012-01-13 09:33:46 PST
Created attachment 133107 [details]
Patch
Comment on attachment 133107 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133107&action=review Good catch, I'd like to r+ this, though the test should be extended to cover text as well, or a new test should be added. Its possible that theres still a bug related to non-shapes drawing in fO trees. > LayoutTests/svg/zoom/page/zoom-foreign-content.svg:2 > +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintTest()"> For the record: in theory this could use a reftest, if we'd zoom the subtrees by specifying zoom CSS properties, though for testing zooming the preferred way is to use the DRT functionality, as that uses the same code path, as if you'd zoom in eg. Safari. So using a pixel+repaint test is just right for this case. > LayoutTests/svg/zoom/page/zoom-foreign-content.svg:7 > + <rect width="99" height="99" fill="red"/> I'd like to see another test that includes text in the SVG subtree (and maybe also in the HTML subtree). As <text> depends on the accumulated transforms up to RenderSVGRoot, we should verify this works as well, including zooming. (In reply to comment #2) > (From update of attachment 133107 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133107&action=review > > Good catch, I'd like to r+ this, though the test should be extended to cover text as well, or a new test should be added. Its possible that theres still a bug related to non-shapes drawing in fO trees. There's already a test covering zooming of FO text: svg/zoom/{page,text}/zoom-foreignObject.svg Should I go ahead and add more coverage to it (SVG text maybe)? I was hoping to avoid having to deal with a gazillion platform rebaselines :) Created attachment 133535 [details]
Patch
Comment on attachment 133535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133535&action=review Looks great, r=me, with one comment: > LayoutTests/platform/mac/Skipped:566 > +# Rebaseline needed after https://bugs.webkit.org/show_bug.cgi?id=76282 > +svg/zoom/page/zoom-foreignObject.svg > +svg/zoom/text/zoom-foreignObject.svg You can use platform/mac/test_expectations.txt as well btw, so there's no need to skip those tests! Created attachment 133635 [details]
Patch
(In reply to comment #5) > You can use platform/mac/test_expectations.txt as well btw, so there's no need to skip those tests! Good to know! Still figuring out which platforms observe the expectations files :) Comment on attachment 133635 [details] Patch Clearing flags on attachment: 133635 Committed r112022: <http://trac.webkit.org/changeset/112022> All reviewed patches have been landed. Closing bug. |