The content='...' parsing error is not a viewport error, so keep it separate

The content='...' parsing error is not a viewport error, so keep it separate

The parsing algorithm for the meta content attribute reports an error
for presense of ';'. It is currently using viewport error reporting, though
it is not tied to the viewport meta tag, but meta tags in general.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157367

[Mike West, 2013-09-06 12:17:47]

https://codereview.chromium.org/24025002/diff/1/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/24025002/diff/1/Source/core/dom/Document.cpp#...
Source/core/dom/Document.cpp:3034: String message = "Note that ';' is not a
key-value pair separator. The list should be comma-separated.";
Since you're changing this anyway, I'd suggest making it clear that the meta tag
is the problem. Something like "Error parsing a meta element's content: ';' is
not a key-value pair separator, etc.".

I think we should do that for the other messages as well, now that I look at
this. But not in this patch. :)

https://codereview.chromium.org/24025002/diff/1/Source/core/dom/Document.cpp#...
Source/core/dom/Document.cpp:3035: addConsoleMessage(OtherMessageSource,
WarningMessageLevel, message);
I believe this should be 'RenderingMessageSource'.

[Mike West, 2013-09-06 12:33:26]

LGTM, with one nit that I should have noted on the first pass (sorry!).

https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.c...
Source/core/dom/Document.cpp:3035: addConsoleMessage(RenderingMessageSource,
WarningMessageLevel, message);
Nit: If you don't mind, just inline this. There's no reason to create the
'message' variable and then throw it away.

[kenneth.r.christiansen, 2013-09-06 12:37:19]

On 2013/09/06 12:33:26, Mike West wrote:
> LGTM, with one nit that I should have noted on the first pass (sorry!).
> 
>
https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.cpp
> File Source/core/dom/Document.cpp (right):
> 
>
https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.c...
> Source/core/dom/Document.cpp:3035: addConsoleMessage(RenderingMessageSource,
> WarningMessageLevel, message);
> Nit: If you don't mind, just inline this. There's no reason to create the
> 'message' variable and then throw it away.

The reason I did it like that is that it is how it is done in many other places,
32 other places according to 'git grep "String message =" Source/core | wc -'
including in Document.cpp.

The compiler should optimize it anyways

[Use mkwst_at_chromium.org plz., 2013-09-06 12:44:59]

On 2013/09/06 12:37:19, kenneth.r.christiansen wrote:
> On 2013/09/06 12:33:26, Mike West wrote:
> > LGTM, with one nit that I should have noted on the first pass (sorry!).
> > 
> >
>
https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.cpp
> > File Source/core/dom/Document.cpp (right):
> > 
> >
>
https://codereview.chromium.org/24025002/diff/5001/Source/core/dom/Document.c...
> > Source/core/dom/Document.cpp:3035: addConsoleMessage(RenderingMessageSource,
> > WarningMessageLevel, message);
> > Nit: If you don't mind, just inline this. There's no reason to create the
> > 'message' variable and then throw it away.
> 
> The reason I did it like that is that it is how it is done in many other
places,
> 32 other places according to 'git grep "String message =" Source/core | wc -'
> including in Document.cpp.
> 
> The compiler should optimize it anyways

*shrug* land it. i'll look at those at some point; i don't think we need the
temporary variable.

[commit-bot: I haz the power, 2013-09-06 12:47:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kenneth.r.christiansen@intel.com/24025...

[commit-bot: I haz the power, 2013-09-06 14:12:21]

Change committed as 157367