Bug 69694 - PluginDocumentParser uses incorrect syntax for background color
Summary: PluginDocumentParser uses incorrect syntax for background color
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nico Weber
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-07 21:58 PDT by Nico Weber
Modified: 2011-10-10 16:31 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.38 KB, patch)
2011-10-07 22:00 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (2.38 KB, patch)
2011-10-08 12:50 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (3.10 KB, patch)
2011-10-08 12:52 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (6.31 KB, patch)
2011-10-09 12:11 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch for landing (5.88 KB, patch)
2011-10-09 13:04 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch for landing (6.79 KB, patch)
2011-10-10 15:10 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch for landing (6.75 KB, patch)
2011-10-10 15:18 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch for landing (6.73 KB, patch)
2011-10-10 15:25 PDT, Nico Weber
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Weber 2011-10-07 21:58:35 PDT
PluginDocument: bgcolor doesn't support rgb() syntax
Comment 1 Nico Weber 2011-10-07 22:00:03 PDT
Created attachment 110256 [details]
Patch
Comment 2 Nico Weber 2011-10-07 22:01:52 PDT
This was last modified here: http://trac.webkit.org/changeset/24242

With this patch, this matches MediaDocument (see http://codesearch.google.com/codesearch#search/&exact_package=chromium&q=rgb%5C(38,38,38%5C)&type=cs ).

Found by ben.quar…@callmeqube.com: http://code.google.com/p/chromium/issues/detail?id=93168#c11
Comment 3 Andreas Kling 2011-10-08 07:12:04 PDT
Comment on attachment 110256 [details]
Patch

Looks sane, but needs a test.
Comment 4 Nico Weber 2011-10-08 09:45:54 PDT
The reason why this is happening now is because of http://trac.webkit.org/changeset/90139/trunk . With that, "rgb(38,38,38)" is converted to "0000380380380" -> "0000380380380" -> "00003 80380 38000" -> "#038038", which is the background green that's now visible for plugins.
Comment 5 Nico Weber 2011-10-08 12:50:54 PDT
Created attachment 110275 [details]
Patch
Comment 6 Nico Weber 2011-10-08 12:52:22 PDT
Created attachment 110276 [details]
Patch
Comment 7 Nico Weber 2011-10-08 12:52:49 PDT
Comment on attachment 110276 [details]
Patch

expected files still missing, i'll add them soon
Comment 8 Nico Weber 2011-10-09 12:11:12 PDT
Created attachment 110305 [details]
Patch
Comment 9 Nico Weber 2011-10-09 12:13:30 PDT
This is http://crbug.com/93168
Comment 10 Nico Weber 2011-10-09 13:04:12 PDT
Created attachment 110308 [details]
Patch for landing
Comment 11 WebKit Review Bot 2011-10-09 14:41:57 PDT
Comment on attachment 110308 [details]
Patch for landing

Rejecting attachment 110308 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
SS
  media/video-playing-and-pause.html = MISSING PASS


Regressions: Unexpected text diff mismatch : (1)
  plugins/iframe-plugin-bgcolor.html = TEXT

Regressions: Unexpected image mismatch : (5)
  fast/text/atsui-multiple-renderers.html = IMAGE
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE



Full output: http://queues.webkit.org/results/10015598
Comment 12 Nico Weber 2011-10-09 16:09:49 PDT
Comment on attachment 110308 [details]
Patch for landing

can't repro that locally. trying again.
Comment 13 WebKit Review Bot 2011-10-09 17:13:17 PDT
Comment on attachment 110308 [details]
Patch for landing

Rejecting attachment 110308 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
SS
  media/video-playing-and-pause.html = MISSING PASS


Regressions: Unexpected text diff mismatch : (1)
  plugins/iframe-plugin-bgcolor.html = TEXT

Regressions: Unexpected image mismatch : (5)
  fast/text/atsui-multiple-renderers.html = IMAGE
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE



Full output: http://queues.webkit.org/results/10009823
Comment 14 Alexey Proskuryakov 2011-10-09 23:04:22 PDT
Bug title and description just say that "bgcolor doesn't support rgb() syntax", which doesn't seem to be what's fixed here.

Would "PluginDocumentParser uses incorrect syntax for background color" correctly describe the bug?
Comment 15 Nico Weber 2011-10-10 15:10:43 PDT
Created attachment 110410 [details]
Patch for landing
Comment 16 Nico Weber 2011-10-10 15:11:59 PDT
The document is 1px higher on linux for some reason, but the pixel test succeeds anyway. Trying with a platform-specific txt expectations file; I opened issue 69791 for getting win/linux baselines.

ap: Thanks, that's way better, done.
Comment 17 WebKit Review Bot 2011-10-10 15:14:29 PDT
Comment on attachment 110410 [details]
Patch for landing

Rejecting attachment 110410 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
Hunk #1 FAILED at 3838.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej
patching file LayoutTests/platform/mac/plugins/iframe-plugin-bgcolor-expected.txt
patching file LayoutTests/plugins/iframe-plugin-bgcolor.html
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/html/PluginDocument.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/10027059
Comment 18 Nico Weber 2011-10-10 15:18:56 PDT
Created attachment 110416 [details]
Patch for landing
Comment 19 WebKit Review Bot 2011-10-10 15:21:06 PDT
Comment on attachment 110416 [details]
Patch for landing

Rejecting attachment 110416 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
Hunk #1 FAILED at 3758.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej
patching file LayoutTests/platform/mac/plugins/iframe-plugin-bgcolor-expected.txt
patching file LayoutTests/plugins/iframe-plugin-bgcolor.html
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/html/PluginDocument.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/10030034
Comment 20 Nico Weber 2011-10-10 15:25:10 PDT
Created attachment 110419 [details]
Patch for landing
Comment 21 WebKit Review Bot 2011-10-10 16:31:23 PDT
Comment on attachment 110419 [details]
Patch for landing

Clearing flags on attachment: 110419

Committed r97103: <http://trac.webkit.org/changeset/97103>
Comment 22 WebKit Review Bot 2011-10-10 16:31:28 PDT
All reviewed patches have been landed.  Closing bug.