Bug 70325 - Web Inspector: CSS background-image applied inline shows a warning, but still works.
Summary: Web Inspector: CSS background-image applied inline shows a warning, but still...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-18 06:35 PDT by Alexander Pavlov (apavlov)
Modified: 2011-10-28 02:43 PDT (History)
11 users (show)

See Also:


Attachments
[PATCH] Suggested fix (27.84 KB, patch)
2011-10-19 04:34 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Added a couple of required drive-by style fixes (28.45 KB, patch)
2011-10-19 04:36 PDT, Alexander Pavlov (apavlov)
pfeldman: review-
Details | Formatted Diff | Diff
[PATCH] Removed whitespace fixes (28.45 KB, patch)
2011-10-19 06:38 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Retry attachment for bots (they should have caught up with the previous commit) (28.45 KB, patch)
2011-10-19 06:56 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Retry, take 2 (6.36 KB, patch)
2011-10-19 07:13 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] ChangeLog entry made more verbose (6.51 KB, patch)
2011-10-19 07:43 PDT, Alexander Pavlov (apavlov)
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2011-10-18 06:35:47 PDT
1. declare a CSS background-image inline (within the style
attribute of a <tag>)

What do you see instead?

Chrome's Developer Tools shows a warning icon
and strikethrough of the rule, yet the image still renders fine. No
warning is shown if the background-image is declared as a CSS rule.

Changing the inline rule from 'background-image' to just
'background' (but leaving only the url value) causes the warning to go
away. This seems very strange and I am curious if it is a bug.

Please see this example where there are 3 background-images (1 CSS
rule, 2 inline). The 2 inline have warnings. Change one of them to be
just 'background' and the warning will go away.

http://jsfiddle.net/sandeep/RDmRz/2/show/


Upstreaming http://code.google.com/p/chromium/issues/detail?id=100646
Comment 1 Alexander Pavlov (apavlov) 2011-10-19 04:34:12 PDT
Created attachment 111591 [details]
[PATCH] Suggested fix

The whitespace fixes are included into this change not a separate one, as suggested by Zoltan on #webkit.
Comment 2 WebKit Review Bot 2011-10-19 04:36:23 PDT
Attachment 111591 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/css/CSSParser.cpp:5689:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/css/CSSParser.cpp:5716:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alexander Pavlov (apavlov) 2011-10-19 04:36:44 PDT
Created attachment 111592 [details]
[PATCH] Added a couple of required drive-by style fixes
Comment 4 Pavel Feldman 2011-10-19 05:11:53 PDT
Comment on attachment 111592 [details]
[PATCH] Added a couple of required drive-by style fixes

View in context: https://bugs.webkit.org/attachment.cgi?id=111592&action=review

> Source/WebCore/css/CSSParser.cpp:557
> +    if (!m_styleSheet)

Please do not mix semantics changes with the lint fixes.

> Source/WebCore/css/CSSParser.h:259
> +    void setStyleSheet(CSSStyleSheet*);

I would add CSSStyleSheet* contextStyleSheet parameter into the parseDeclaration and make existing call sites (~3 of them) explicitly pass either parent stylesheet or 0 there.
Comment 5 Alexander Pavlov (apavlov) 2011-10-19 06:38:22 PDT
Created attachment 111606 [details]
[PATCH] Removed whitespace fixes
Comment 6 Alexander Pavlov (apavlov) 2011-10-19 06:56:30 PDT
Created attachment 111608 [details]
[PATCH] Retry attachment for bots (they should have caught up with the previous commit)
Comment 7 Alexander Pavlov (apavlov) 2011-10-19 07:13:49 PDT
Created attachment 111612 [details]
[PATCH] Retry, take 2
Comment 8 WebKit Review Bot 2011-10-19 07:15:48 PDT
Attachment 111612 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/css/CSSParser.cpp:551:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:73:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Alexander Pavlov (apavlov) 2011-10-19 07:17:16 PDT
(In reply to comment #8)
> Attachment 111612 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1
> 
> Source/WebCore/css/CSSParser.cpp:551:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
> Source/WebCore/css/CSSParser.h:73:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
> Total errors found: 2 in 7 files

These are false positives (AFAIK, the lint fix is in the works).
Comment 10 Yury Semikhatsky 2011-10-19 07:34:23 PDT
Comment on attachment 111612 [details]
[PATCH] Retry, take 2

View in context: https://bugs.webkit.org/attachment.cgi?id=111612&action=review

Inspector change looks good.

> LayoutTests/ChangeLog:5
> +

Please describe the change here.
Comment 11 Alexander Pavlov (apavlov) 2011-10-19 07:43:34 PDT
Created attachment 111617 [details]
[PATCH] ChangeLog entry made more verbose
Comment 12 WebKit Review Bot 2011-10-19 07:45:51 PDT
Attachment 111617 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/css/CSSParser.cpp:551:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:73:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Antti Koivisto 2011-10-28 02:11:29 PDT
Comment on attachment 111617 [details]
[PATCH] ChangeLog entry made more verbose

r=me but please fix the style.
Comment 14 Alexander Pavlov (apavlov) 2011-10-28 02:30:17 PDT
(In reply to comment #13)
> (From update of attachment 111617 [details])
> r=me but please fix the style.

The style issues are due to a stylebot bug, filed as bug 71101.
Comment 15 Alexander Pavlov (apavlov) 2011-10-28 02:43:54 PDT
Committed r98714: <http://trac.webkit.org/changeset/98714>