Bug 20651

Created attachment 23179 [details] Minimal test case

Forgot to mention - attached test case works in FireFox (box is green), and fails in WebKit nightly as of 4th september (box is red).

This test case now crashes WebKit when opened - here's the debugging info: http://pastie.org/266285

As Eric pointed out, the code (surprisingly) exhibits the correct methods for accessing/modifying the class. They just don't work d-: Catfish_Man pointed out in IRC that the element does, in fact, show the new class in the Web Inspector DOM inspector, as well as in the Properties field - it does not, however, show the new class in the Styles pane, and the class doesn't apply in the actual document either.

I have a patch to fix this on my work machine.

(In reply to comment #6) > I have a patch to fix this on my work machine. > Cool, cool, looking forward to seeing it committed [-:

Created attachment 24124 [details] Potential fix for element.className.baseValue LayoutTests/ChangeLog | 9 ++++ .../svg/custom/class-baseValue-expected.checksum | 1 + .../mac/svg/custom/class-baseValue-expected.png | Bin 0 -> 3250 bytes .../mac/svg/custom/class-baseValue-expected.txt | 5 ++ LayoutTests/svg/custom/class-baseValue.svg | 10 +++++ WebCore/ChangeLog | 14 +++++++ WebCore/dom/StyledElement.cpp | 41 ++++++++++--------- WebCore/dom/StyledElement.h | 5 ++ WebCore/svg/SVGStyledElement.cpp | 3 + 9 files changed, 69 insertions(+), 19 deletions(-)

Comment on attachment 24124 [details] Potential fix for element.className.baseValue Can the test be changed into a dumpAsText test? I was able to do a number of SVG tests that were portable that way. + if (attrName == HTMLNames::classAttr) I'm surprised you chose to explicitly qualify here rather than doing using namespace HTMLNames. StyledElement::classAttributeChanged would be better if it only called setChanged when there was an actual change. But that's not part of this fix, I guess. r=me

(In reply to comment #9) > (From update of attachment 24124 [details] [edit]) > Can the test be changed into a dumpAsText test? I was able to do a number of > SVG tests that were portable that way. I didn't do so, but it probably could have been. :( Honestly I somehow missed this line of your comment before commiting > + if (attrName == HTMLNames::classAttr) > > I'm surprised you chose to explicitly qualify here rather than doing using > namespace HTMLNames. This was intentional, given that we're in an SVG file. > StyledElement::classAttributeChanged would be better if it only called > setChanged when there was an actual change. But that's not part of this fix, I > guess. I built a test which toggled classNames on an element to equivilent versions, I didn't see extra redraws, but we might have been doing extra style resolutions? Or maybe my test was busted. <style> .foo { background-color: green; } .bar { width: 100px; height: 100px; } </style> <div id="test" class="foo bar"></div> <script> var toggleCount = 0; function toggleClassNames() { var div = document.getElementById("test"); if (toggleCount % 2) { div.className = "foo bar"; } else { div.className = " bar foo "; } setTimeout(100, toggleClassNames); } toggleClassNames(); </script>

Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/platform/mac/svg/custom/class-baseValue-expected.checksum A LayoutTests/platform/mac/svg/custom/class-baseValue-expected.png A LayoutTests/platform/mac/svg/custom/class-baseValue-expected.txt A LayoutTests/svg/custom/class-baseValue.svg M WebCore/ChangeLog M WebCore/dom/StyledElement.cpp M WebCore/dom/StyledElement.h M WebCore/svg/SVGStyledElement.cpp Committed r37391

Here is a fully functioning test case which still demonstrates that setChanged() doesn't cause a redraw and thus doesn't seem to be a big worry here re: classes being set to the same value causing too much work. <style> .foo { background-color: green; } .bar { width: 100px; height: 100px; } .baz { background-color: red; } </style> <div id="test" class="foo bar"></div> <script> var toggleCount = 0; function toggleClassNames() { var div = document.getElementById("test"); if (toggleCount++ % 2) { div.className = "foo bar"; } else { div.className = " bar foo "; } setTimeout("toggleClassNames()", 100); } toggleClassNames(); </script>

Rolled out in r37401 since it introduced 46 regression test failures.

Damn, and I was excited to see it in tomorrow's nightly d-:

Created attachment 24164 [details] Fix the regression and avoid double-parse WebCore/svg/SVGStyledElement.cpp | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)

Created attachment 24167 [details] Forgot to check in these changes in my last checkin. * rendering/RenderThemeSafari.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29265 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 6 ++++++ WebCore/rendering/RenderThemeSafari.cpp | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)

Created attachment 24168 [details] WebCore: Reviewed by Sam Weinig Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523 <rdar://problem/5657447> When a frame is created with the URL "about:blank" or "", it should inherit its SecurityOrigin from its opener. However, once it has decided on that SecurityOrigin, it should not change its mind. Prior to this patch, several events could induce the frame to change its SecurityOrigin, permitting an attacker to inject script into an arbitrary SecurityOrigin. This patch makes several changes: 1) Documents refuse to change from one SecurityOrigin to another unless explicitly instructed to do so. 2) Navigating to a JavaScript URL that produces a value preserves the current SecurityOrigin explicitly instead of relying on the URL to preserve the origin (which fails for about:blank URLs and SecurityOrigins with document.domain set). Ideally, we should not preserve the URL at all. Instead, the frame's URL should be the JavaScript URL, as in Firefox, but this would require changes that are too risky for this patch. I'll file this as a separate issue. 3) Various methods of navigating to JavaScript URLs were not properly handling JavaScript that returned a value (and should therefore replace the current document). This patch unifies those code paths with the path that works. There are still a handful of bugs relating to the handling of JavaScript URLs, but I'll file those as separate issues. Tests: http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html http/tests/security/aboutBlank/xss-DENIED-set-opener.html * dom/Document.cpp: (WebCore::Document::initSecurityOrigin): * dom/Document.h: (WebCore::Document::setSecurityOrigin): * loader/FrameLoader.cpp: (WebCore::FrameLoader::changeLocation): (WebCore::FrameLoader::urlSelected): (WebCore::FrameLoader::requestFrame): (WebCore::FrameLoader::submitForm): (WebCore::FrameLoader::executeIfJavaScriptURL): (WebCore::FrameLoader::begin): * loader/FrameLoader.h: * platform/SecurityOrigin.cpp: (WebCore::SecurityOrigin::setForURL): (WebCore::SecurityOrigin::createForFrame): * platform/SecurityOrigin.h: LayoutTests: Reviewed by Sam Weinig. Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523 Adds new LayoutTests for scripting from about:blank windows. These windows should inherit its SecurityOrigin from its opener and should refuse to change their origins when their opener changes exogenously (the navigate-opener tests) or explicitly (the set-opener test). * http/tests/security/aboutBlank: Added. * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write-expected.txt: Added. * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html: Added. * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expected.txt: Added. * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html: Added. * http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt: Added. * http/tests/security/aboutBlank/xss-DENIED-set-opener.html: Added. * http/tests/security/resources/innocent-victim-with-notify.html: Added. * http/tests/security/resources/innocent-victim.html: Added. * http/tests/security/resources/libwrapjs.js: Added. * http/tests/security/resources/open-window.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29266 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 23 ++++ ...IED-navigate-opener-document-write-expected.txt | 17 +++ .../xss-DENIED-navigate-opener-document-write.html | 105 +++++++++++++++++++ ...IED-navigate-opener-javascript-url-expected.txt | 17 +++ .../xss-DENIED-navigate-opener-javascript-url.html | 106 ++++++++++++++++++++ .../aboutBlank/xss-DENIED-set-opener-expected.txt | 20 ++++ .../security/aboutBlank/xss-DENIED-set-opener.html | 76 ++++++++++++++ .../resources/innocent-victim-with-notify.html | 14 +++ .../tests/security/resources/innocent-victim.html | 5 + .../http/tests/security/resources/libwrapjs.js | 62 ++++++++++++ .../http/tests/security/resources/open-window.html | 22 ++++ LayoutTests/platform/win/Skipped | 3 + WebCore/ChangeLog | 58 +++++++++++ WebCore/dom/Document.cpp | 3 + WebCore/dom/Document.h | 5 + WebCore/loader/FrameLoader.cpp | 53 +++++----- WebCore/loader/FrameLoader.h | 8 +- WebCore/platform/SecurityOrigin.cpp | 55 ++++------ WebCore/platform/SecurityOrigin.h | 7 +- 19 files changed, 592 insertions(+), 67 deletions(-)

Created attachment 24169 [details] Build fix. * platform/graphics/qt/SimpleFontDataQt.cpp: (WebCore::SimpleFontData::SimpleFontData): (WebCore::SimpleFontData::~SimpleFontData): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29267 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 8 ++++++++ WebCore/platform/graphics/qt/SimpleFontDataQt.cpp | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-)

Created attachment 24170 [details] Reviewed by Mark. Enable SVG_FONTS by default. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29268 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- JavaScriptCore/ChangeLog | 8 ++++++++ .../Configurations/JavaScriptCore.xcconfig | 2 +- WebCore/ChangeLog | 9 +++++++++ WebCore/Configurations/WebCore.xcconfig | 2 +- WebCore/WebCore.vcproj/build-generated-files.sh | 2 +- WebKit/mac/ChangeLog | 8 ++++++++ WebKit/mac/Configurations/WebKit.xcconfig | 2 +- 7 files changed, 29 insertions(+), 4 deletions(-)

Created attachment 24171 [details] Build fix. Need to wrap these classes in #if ENABLE(VIDEO) * rendering/MediaControlElements.cpp: * rendering/MediaControlElements.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29269 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 7 +++++++ WebCore/rendering/MediaControlElements.cpp | 4 ++++ WebCore/rendering/MediaControlElements.h | 4 +++- 3 files changed, 14 insertions(+), 1 deletions(-)

Created attachment 24172 [details] 2008-01-07 Jon Honeycutt <jhoneycutt@apple.com> Reviewed by Hyatt. <rdar://problem/5673489> Safari does not render windowless plugins in an iframe when opacity < 1.0 Plugins in transparency layers handle their own world transforms, so only apply the horizontal/vertical transform if we are not in a transparency layer. * platform/graphics/GraphicsContext.h: Add a Windows-platform-only inTransparencyLayer() function * platform/win/GraphicsContextWin.cpp: (WebCore::GraphicsContext::getWindowsContext): Use inTransparencyLayer() (WebCore::GraphicsContext::inTransparencyLayer): (WebCore::GraphicsContext::releaseWindowsContext): Use inTransparencyLayer() * plugins/win/PluginViewWin.cpp: (WebCore::PluginViewWin::paint): When retrieving the HDC, use the rect relative to the window. Pass m_isTransparent to get/releaseWindowsContext(). Only set the world transform if we are not in a transparency layer. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29270 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 24 ++++++++++++++++++++++++ WebCore/platform/graphics/GraphicsContext.h | 1 + WebCore/platform/win/GraphicsContextWin.cpp | 6 ++++-- WebCore/plugins/win/PluginViewWin.cpp | 27 +++++++++++++-------------- 4 files changed, 42 insertions(+), 16 deletions(-)

Created attachment 24173 [details] Add missing newline. * rendering/MediaControlElements.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29271 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 6 ++++++ WebCore/rendering/MediaControlElements.cpp | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-)

Created attachment 24174 [details] Build fix. * WebKit.vcproj/InterfacesGenerated.vcproj: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29272 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebKit/win/ChangeLog | 6 ++++++ .../win/WebKit.vcproj/InterfacesGenerated.vcproj | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-)

Created attachment 24175 [details] Build fix. * platform/graphics/qt/GlyphPageTreeNodeQt.cpp: (WebCore::GlyphPageTreeNode::pruneTreeCustomFontData): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29273 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 7 +++++++ .../platform/graphics/qt/GlyphPageTreeNodeQt.cpp | 2 +- 2 files changed, 8 insertions(+), 1 deletions(-)

Created attachment 24176 [details] Fix painting of SVG <image> when the image must be scaled to retain aspect ratio Reviewed by Niko Also added new layout test for this bug, and corrected old expected output git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29275 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 25 ++++++++++++++++++++ .../animate-elem-30-t-expected.checksum | 2 +- .../svg/W3C-SVG-1.1/animate-elem-30-t-expected.png | Bin 20612 -> 20691 bytes .../animate-elem-36-t-expected.checksum | 2 +- .../svg/W3C-SVG-1.1/animate-elem-36-t-expected.png | Bin 32491 -> 32406 bytes .../animate-elem-39-t-expected.checksum | 2 +- .../svg/W3C-SVG-1.1/animate-elem-39-t-expected.png | Bin 37743 -> 38282 bytes .../svg/carto.net/selectionlist-expected.checksum | 2 +- .../mac/svg/carto.net/selectionlist-expected.png | Bin 208607 -> 208824 bytes ...age-with-aspect-ratio-stretch-expected.checksum | 1 + .../image-with-aspect-ratio-stretch-expected.png | Bin 0 -> 3250 bytes .../image-with-aspect-ratio-stretch-expected.txt | 11 ++++++++ .../custom/pointer-events-image-expected.checksum | 2 +- .../svg/custom/pointer-events-image-expected.png | Bin 71136 -> 68766 bytes .../custom/text-image-opacity-expected.checksum | 2 +- .../mac/svg/custom/text-image-opacity-expected.png | Bin 17588 -> 17393 bytes .../svg/custom/image-with-aspect-ratio-stretch.svg | 17 +++++++++++++ WebCore/ChangeLog | 11 ++++++++ WebCore/rendering/RenderSVGImage.cpp | 16 ++++++------ 19 files changed, 79 insertions(+), 14 deletions(-)

Created attachment 24177 [details] Temporary results until I fix: <rdar://problem/5674667> fast/forms/slider-mouse-events.html is broken by media control checkin 29257 * fast/forms/slider-mouse-events-expected.txt: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29276 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 7 +++++++ .../fast/forms/slider-mouse-events-expected.txt | 1 - 2 files changed, 7 insertions(+), 1 deletions(-)

Created attachment 24178 [details] * platform/win/Skipped: removing fixed test git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29277 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 5 +++++ LayoutTests/platform/win/Skipped | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-)

Created attachment 24179 [details] Build fix. * WebCore.vcproj/WebCore.sln: * WebCore.vcproj/WebCore.submit.sln: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29278 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 7 +++++++ WebCore/WebCore.vcproj/WebCore.sln | 9 +++++++++ WebCore/WebCore.vcproj/WebCore.submit.sln | 9 +++++++++ 3 files changed, 25 insertions(+), 0 deletions(-)

Created attachment 24180 [details] Full fix (no layout test regressions) I am *so so so* sorry for the list spam. git-send-bugzilla freaked out a bit. :(

(In reply to comment #29) > Created an attachment (id=24180) [edit] > Full fix (no layout test regressions) > > I am *so so so* sorry for the list spam. git-send-bugzilla freaked out a bit. > :( > I was gonna say, this doesn't exactly look related to the bug in question... hah! <3 git though (-:

Comment on attachment 24180 [details] Full fix (no layout test regressions) Oops, good catch :-) r=me. .oO(I thought you ran layout tests before ;-)

I thought I did to. Clearly I was wrong.