WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
201536
Test SVGViewSpec behavior across page-loads with different anchors
https://bugs.webkit.org/show_bug.cgi?id=201536
Summary
Test SVGViewSpec behavior across page-loads with different anchors
Nikolas Zimmermann
Reported
2019-09-06 00:55:06 PDT
Split off a layout test from
bug 94469
, that is unrelated to SMIL animations of the SVGViewElement, but helpful to document the expected SVGViewElement/SVGViewSpec behavior.
Attachments
Patch
(13.09 KB, patch)
2019-09-06 00:55 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch
(12.86 KB, patch)
2019-09-09 13:55 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch
(12.85 KB, patch)
2019-09-10 03:00 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch
(12.79 KB, patch)
2019-09-10 11:29 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1
2019-09-06 00:55:46 PDT
Created
attachment 378165
[details]
Patch
Nikolas Zimmermann
Comment 2
2019-09-06 06:33:11 PDT
The failure in the ios-wk2 EWS bot is unrelated to the patch.
Rob Buis
Comment 3
2019-09-09 04:20:23 PDT
Comment on
attachment 378165
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=378165&action=review
> LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:23 > + document.body.appendChild(script);
Why is the js-test-post.js script added dynamically at the end? Could it not be declared right after js-test-pre.js?
> LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:173 > +iframeElement.setAttribute("style", "display: block");
Is this needed?
Nikolas Zimmermann
Comment 4
2019-09-09 05:13:01 PDT
(In reply to Rob Buis from
comment #3
) Thanks Rob!
> > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:23 > > + document.body.appendChild(script); > > Why is the js-test-post.js script added dynamically at the end? Could it not > be declared right after js-test-pre.js?
Indeed, there is a 'jsTestIsAsync' mode that I can use. But still it cannot be loaded right after js-test-pre.js, because I need a chance to say "jsTestIsAsync = true", which is typically done after the description() call, when comparing to other tests. But I should be able to get rid of the custom completeJSTest() method: avoiding boilerplate is nice, so I will try that.
> > > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:173 > > +iframeElement.setAttribute("style", "display: block"); > > Is this needed?
I will see how it looks in MiniBrowser w/o that. For the actual test result, it makes no difference.
Rob Buis
Comment 5
2019-09-09 05:34:58 PDT
> > > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:173 > > > +iframeElement.setAttribute("style", "display: block"); > > > > Is this needed? > > I will see how it looks in MiniBrowser w/o that. For the actual test result, > it makes no difference.
From my testing display inline will be used, but indeed if it does not change the test it is better to remove unneeded changes.
Nikolas Zimmermann
Comment 6
2019-09-09 13:53:24 PDT
(In reply to Rob Buis from
comment #5
)
> > > > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:173 > > > > +iframeElement.setAttribute("style", "display: block"); > > > > > > Is this needed? > > > > I will see how it looks in MiniBrowser w/o that. For the actual test result, > > it makes no difference. > > From my testing display inline will be used, but indeed if it does not > change the test it is better to remove unneeded changes.
Visually there is no difference, I removed the line it is not necessary. Please check the new patch :-)
Nikolas Zimmermann
Comment 7
2019-09-09 13:55:34 PDT
Created
attachment 378405
[details]
Patch
Rob Buis
Comment 8
2019-09-10 00:05:11 PDT
Comment on
attachment 378405
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=378405&action=review
> LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:14 > + testRunner.waitUntilDone();
This should not be needed anymore.
> LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:73 > + debug("Verify that no load was peformed, but only a different view was set on the same document");
performed.
> LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:118 > + debug("Verify that no load was peformed, but only a different view was set on the same document");
Ditto.
Nikolas Zimmermann
Comment 9
2019-09-10 02:58:38 PDT
(In reply to Rob Buis from
comment #8
)
> Comment on
attachment 378405
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=378405&action=review
> > > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:14 > > + testRunner.waitUntilDone(); > > This should not be needed anymore.
Indeed, good catch. This framework has changed since I looked at it last time :-)
> > > LayoutTests/svg/dom/SVGViewSpec-multiple-views.html:73 > > + debug("Verify that no load was peformed, but only a different view was set on the same document"); > > performed.
Done.
Nikolas Zimmermann
Comment 10
2019-09-10 03:00:23 PDT
Created
attachment 378451
[details]
Patch
Rob Buis
Comment 11
2019-09-10 07:19:40 PDT
Comment on
attachment 378451
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=378451&action=review
> LayoutTests/ChangeLog:10 > + navigating between anchors, which indicates that no new document was created. Verify that the SVGViewSpec API works as intended, across navigations.
Please try to stick to max 100 characters here.
Nikolas Zimmermann
Comment 12
2019-09-10 10:51:40 PDT
(In reply to Rob Buis from
comment #11
)
> Please try to stick to max 100 characters here.
Ok. Shall I upload a new patch? If not, how shall we land it? Will you set cq+?
Rob Buis
Comment 13
2019-09-10 11:19:05 PDT
(In reply to Nikolas Zimmermann from
comment #12
)
> (In reply to Rob Buis from
comment #11
) > > Please try to stick to max 100 characters here. > Ok. Shall I upload a new patch? If not, how shall we land it? Will you set > cq+?
Sure, upload a new patch fixing the width and I will cq+.
Nikolas Zimmermann
Comment 14
2019-09-10 11:29:10 PDT
Created
attachment 378470
[details]
Patch
Nikolas Zimmermann
Comment 15
2019-09-10 12:00:09 PDT
Thanks Rob! Have a good night.
WebKit Commit Bot
Comment 16
2019-09-10 12:57:28 PDT
Comment on
attachment 378470
[details]
Patch Clearing flags on attachment: 378470 Committed
r249730
: <
https://trac.webkit.org/changeset/249730
>
WebKit Commit Bot
Comment 17
2019-09-10 12:57:29 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 18
2019-09-10 12:58:18 PDT
<
rdar://problem/55232996
>
Radar WebKit Bug Importer
Comment 19
2019-09-10 12:58:18 PDT
<
rdar://problem/55232995
>
Nikolas Zimmermann
Comment 20
2019-09-10 13:26:10 PDT
Oops, the title of the bug in the ChangeLog is wrong, it should read "Test SVGViewSpec behavior across page-loads with different anchors" instead of "SMIL animations of SVG <view> element have no effect".
Nikolas Zimmermann
Comment 21
2019-09-17 12:36:00 PDT
(In reply to Nikolas Zimmermann from
comment #20
)
> Oops, the title of the bug in the ChangeLog is wrong, it should read "Test > SVGViewSpec behavior across page-loads with different anchors" instead of > "SMIL animations of SVG <view> element have no effect".
For the record: that was fixed already.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug