The web inspector never shows background-position as part of the background shorthand property. (Bug #20264 is probably a consequence of this.) Minimal HTML to recreate: <title>Web Inspector background-position test</title> <style>p{background:url(#) no-repeat 2px -3px}</style> <p>inspect this element</p>
Created attachment 38503 [details] A Fix The patch depends on the patch for https://bugs.webkit.org/show_bug.cgi?id=26541 CSS property "background-position" was not serialized into the shorthand string value of "style.background". This is because position is stored into two (non-standard?) properties background-position-x and background-position-y. Since "style.backgroundPosition" does serialize as "position-x, position-y", (e.g. 50% 50%) it made sense that the shorthand value of style.background will do the same. I fixed the layout test fast/dom/background-shorthand-csstext.html and fast/dom/background-shorthand-expected.txt as the later was accepting a FAIL.
Comment on attachment 38503 [details] A Fix Do we have a specific section of the spec which talks about this? Ideally we'd have a link to that section in the bug or the ChangeLog.
Comment on attachment 38503 [details] A Fix Rejecting patch 38503 from commit-queue. This patch will require manual commit. Patch https://bugs.webkit.org/attachment.cgi?id=38503 from bug 25489 failed to download and apply.
Comment on attachment 38503 [details] A Fix patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/dom/background-shorthand-csstext-expected.txt Hunk #1 FAILED at 2. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/fast/dom/background-shorthand-csstext-expected.txt.rej patch -p0 "LayoutTests/fast/dom/background-shorthand-csstext-expected.txt" returned 1. Pass --force to ignore patch failures.
Comment on attachment 38503 [details] A Fix http://www.w3.org/TR/2008/WD-css3-background-20080910/#the-background-position According to the spec's examples, background-position should indeed serialize as two numbers. In WebKit, these numbers are stored in background-position-x and background-position-y, but the user will never know. > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > index fa3b8ef..b8674a2 100644 > --- a/LayoutTests/ChangeLog > +++ b/LayoutTests/ChangeLog > @@ -1,3 +1,16 @@ > +2009-08-24 Yaar Schnitman <yaar@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + This patch depends on the patch for bug #26541. > + > + CSS property background-position was not serialized in shorthand string. The layout test accepted that failure, so I fixed them too. > + > + https://bugs.webkit.org/show_bug.cgi?id=25489 > + > + * fast/dom/background-shorthand-csstext-expected.txt: > + * fast/dom/background-shorthand-csstext.html: > + > 2009-08-24 Gustavo Noronha Silva <gustavo.noronha@collabora.co.uk> > > Unreviewed. Skip again tests that we enabled, since they are > diff --git a/LayoutTests/fast/dom/background-shorthand-csstext-expected.txt b/LayoutTests/fast/dom/background-shorthand-csstext-expected.txt > index cc4b3f2..13a79d1 100644 > --- a/LayoutTests/fast/dom/background-shorthand-csstext-expected.txt > +++ b/LayoutTests/fast/dom/background-shorthand-csstext-expected.txt > @@ -2,4 +2,4 @@ This page tests whether or not the background shorthand properly omits initial v > > PASS: document.body.style.background == 'green' should be true and is. > PASS: document.getElementById('div1').style.background == 'repeat-x, white repeat-y' should be true and is. > -FAIL: document.getElementById('div2').style.background == '50% 50% blue' should be true but instead is false. > +PASS: document.getElementById('div2').style.background == 'blue 50% 50%' should be true and is. > diff --git a/LayoutTests/fast/dom/background-shorthand-csstext.html b/LayoutTests/fast/dom/background-shorthand-csstext.html > index f633742..7f1d638 100644 > --- a/LayoutTests/fast/dom/background-shorthand-csstext.html > +++ b/LayoutTests/fast/dom/background-shorthand-csstext.html > @@ -33,7 +33,7 @@ function test() > > shouldBe("document.body.style.background == 'green'", true); > shouldBe("document.getElementById('div1').style.background == 'repeat-x, white repeat-y'", true); > - shouldBe("document.getElementById('div2').style.background == '50% 50% blue'", true); > + shouldBe("document.getElementById('div2').style.background == 'blue 50% 50%'", true); > } > </script> > </head> > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > index a42b457..cfc5b87 100644 > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,3 +1,16 @@ > +2009-08-24 Yaar Schnitman <yaar@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + This patch depends on the patch for bug #26541. > + > + CSS property background-position was not serialized in shorthand string. The layout test accepted that failure, so I fixed them too. > + > + https://bugs.webkit.org/show_bug.cgi?id=25489 > + > + * css/CSSMutableStyleDeclaration.cpp: > + (WebCore::CSSMutableStyleDeclaration::getPropertyValue): > + > 2009-08-24 Nate Chapin <japhet@chromium.org> > > Reviewed by Eric Seidel. > diff --git a/WebCore/css/CSSMutableStyleDeclaration.cpp b/WebCore/css/CSSMutableStyleDeclaration.cpp > index 85ff5e0..0d0235e 100644 > --- a/WebCore/css/CSSMutableStyleDeclaration.cpp > +++ b/WebCore/css/CSSMutableStyleDeclaration.cpp > @@ -115,10 +115,15 @@ String CSSMutableStyleDeclaration::getPropertyValue(int propertyID) const > return getLayeredShorthandValue(properties, 2); > } > case CSSPropertyBackground: { > - const int properties[7] = { CSSPropertyBackgroundColor, CSSPropertyBackgroundImage, CSSPropertyBackgroundRepeat, > - CSSPropertyBackgroundAttachment, CSSPropertyBackgroundPosition, CSSPropertyBackgroundClip, > + const int properties[8] = { CSSPropertyBackgroundColor, > + CSSPropertyBackgroundImage, > + CSSPropertyBackgroundRepeat, > + CSSPropertyBackgroundAttachment, > + CSSPropertyBackgroundPositionX, > + CSSPropertyBackgroundPositionY, > + CSSPropertyBackgroundClip, > CSSPropertyBackgroundOrigin }; > - return getLayeredShorthandValue(properties, 7); > + return getLayeredShorthandValue(properties, 8); > } > case CSSPropertyBorder: { > const int properties[3][4] = {{ CSSPropertyBorderTopWidth,
My understanding of the status of this bug is that the patch has rotted and thus cannot easily be landed as-is. If someone could post an updated patch we can get this r+'d and cq+'d again.
Created attachment 39006 [details] fixes .../dom/background-shorthand-csstext-expected.txt | 4 ++-- .../fast/dom/background-shorthand-csstext.html | 4 ++-- WebCore/css/CSSMutableStyleDeclaration.cpp | 14 ++++++++++---- 3 files changed, 14 insertions(+), 8 deletions(-)
Comment on attachment 39006 [details] fixes Ignore. git-send-bugzilla failed to upload this right
Created attachment 39007 [details] Fixes this bug and 26541 as well
FYI, "bugzilla-tool post-commits" is a drop-in replacement for git-send-bugzilla. Two things to note: 1. post-commits marks review by default, pass --no-review to disable 2. post-commits cherry-picks by default, if you want to attach a range of patches you have to specify it. "HEAD~3" will just attach one patch, not 3. :)
Comment on attachment 39007 [details] Fixes this bug and 26541 as well LGTM.
Comment on attachment 39007 [details] Fixes this bug and 26541 as well Rejecting patch 39007 from commit-queue. This patch will require manual commit. Failed to run "['git', 'svn', 'dcommit']" exit_code: 1 cwd: None
What's wrong with the patch?
Comment on attachment 39007 [details] Fixes this bug and 26541 as well Race condition during commit. ChangeLog was out of date by the time it went to commit. Trying again.
Comment on attachment 39007 [details] Fixes this bug and 26541 as well Clearing flags on attachment: 39007 Committed r48040: <http://trac.webkit.org/changeset/48040>
All reviewed patches have been landed. Closing bug.