Styles are showing as implicit (dimmed) when they shouldn't. Steps: 1) Visit webkit.org 2) Inspect #icon 3) Look at the styles. Results: Multiple things are dimmed. Expected: Nothing should be dimmed here.
Created attachment 39066 [details] Bug
Created attachment 39067 [details] Expected
This regression was introduced with the fix for https://bugs.webkit.org/show_bug.cgi?id=28635.
Created attachment 39914 [details] patch and test
Comment on attachment 39914 [details] patch and test Looks OK to me, but Hyatt, Beth or Mitz should review.
I still wonder why the Inspector shows them as implicit (dim), when they should show as normal properties. Fixing the shorthand is correct. But they should be black despite fixing the shorthand.
(In reply to comment #6) > I still wonder why the Inspector shows them as implicit (dim), when they should > show as normal properties. Fixing the shorthand is correct. But they should be > black despite fixing the shorthand. With the patch applied only background-repeat-x and background-repeat-y are dimmed for the icon at webkit.org while background-repeat is shown as explicit style. It seems to be correct representation since http://webkit.org/css/main.css declares background-repeat: no-repeat; (shorthand property).
(In reply to comment #7) > (In reply to comment #6) > > I still wonder why the Inspector shows them as implicit (dim), when they should > > show as normal properties. Fixing the shorthand is correct. But they should be > > black despite fixing the shorthand. > > With the patch applied only background-repeat-x and background-repeat-y are > dimmed for the icon at webkit.org while background-repeat is shown as explicit > style. It seems to be correct representation since > http://webkit.org/css/main.css declares background-repeat: no-repeat; > (shorthand property). OK, I see. Actually background-repeat-x should be black, since it is the first in the shorthand. Like what you see with margin: 0, margin-top: 0 will be black and the rest are implicit (dim). So I think there is more that needs fixed here, but I don't know what to suggest.
Timothy, what's the status of this patch? Should we land this one and then fix the remaining cases in another bug? Discussion seems to have stagnated.
Comment on attachment 39914 [details] patch and test Fix the ChangeLog typo... "correct" not "cirrect".
(In reply to comment #9) > Timothy, what's the status of this patch? Should we land this one and then fix > the remaining cases in another bug? Discussion seems to have stagnated. Sorry, I was distracted by other issues and didn't have time to work on this one. (In reply to comment #8) > Actually background-repeat-x should be black, since it is the first in the > shorthand. Like what you see with margin: 0, margin-top: 0 will be black and > the rest are implicit (dim). > > So I think there is more that needs fixed here, but I don't know what to > suggest. I don't know whether the difference in handling background-repeat and other properties affected by this patch and those whose values are parsed by CSSParser::parse4Values (margin, padding, color, border-style, border-width) is intentional or not but it is parse4Values that makes first longhand property explicit in some cases. I could support similar behavior for background-repeat and others but it's going to be a bit trickier since they are handled in several places in CSSParser.
Comment on attachment 39914 [details] patch and test Hyatt asked for some minor changes to this patch, so we can't land it automatically.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/backgrounds/repeat/background-repeat-shorthand-expected.txt A LayoutTests/fast/backgrounds/repeat/background-repeat-shorthand.html A LayoutTests/fast/backgrounds/repeat/resources/background-repeat-shorthand.css A LayoutTests/fast/backgrounds/repeat/resources/background-repeat-shorthand.js M WebCore/ChangeLog M WebCore/css/CSSParser.cpp Committed r49616