Bug 33478 - Web Inspector shouldn't show a white color box for "-webkit-line-break: after-white-space"
Summary: Web Inspector shouldn't show a white color box for "-webkit-line-break: after...
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: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-11 10:47 PST by Alexey Proskuryakov
Modified: 2010-02-27 23:10 PST (History)
7 users (show)

See Also:


Attachments
screenshot (21.53 KB, image/png)
2010-01-11 10:47 PST, Alexey Proskuryakov
no flags Details
[PATCH] Smarter Regex (1.38 KB, patch)
2010-02-11 19:48 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[TEST CASE] Element With Offending Style (205 bytes, text/html)
2010-02-11 19:49 PST, Joseph Pecoraro
no flags Details
[PATCH] Proposed Fix (1.38 KB, patch)
2010-02-13 10:19 PST, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-01-11 10:47:54 PST
Created attachment 46290 [details]
screenshot

And if I click on that white box, I get values like "after-hsl(0, 100%, 100%)-space;"!
Comment 1 Joseph Pecoraro 2010-02-11 19:48:32 PST
Created attachment 48612 [details]
[PATCH] Smarter Regex

These two lines from StylesSidebarPane are where the action is:

> var colorRegex = /((?:rgb|hsl)a?\([^)]+\)|#[0-9a-fA-F]{6}|#[0-9a-fA-F]{3}|\b\w+\b)/g;
> var colorProcessor = processValue.bind(window, colorRegex, processColor, null);

The colorRegex's "\b\w+\b" breaks down "after-white-space" and pulls out the "white" in the middle. My simple solution was to throw a negative lookahead at the end of this section ensuring there isn't a "-" after this text. But I want to field you guys for some other possible edge cases, or solutions. My simple patch is attached.
Comment 2 Joseph Pecoraro 2010-02-11 19:49:36 PST
Created attachment 48613 [details]
[TEST CASE] Element With Offending Style
Comment 3 Joseph Pecoraro 2010-02-13 10:19:02 PST
Created attachment 48708 [details]
[PATCH] Proposed Fix

I looked into cases of what could appear after a color in a valid value:

  - '$' for cases like "color: red;" where the value is "red"
  - ')' for cases like -webkit-gradiant's "from(red)", "to", and "color-stop"

These can be put into a positive lookahead at the end of the regex. Timothy Hatcher mentioned whitespace and a comma, which I think are acceptable to add in even though I can't think of cases for them yet.

This lookahead assertion can be used for all cases of colors, but is only practically useful for the \b\w+\b word section, so the patch only uses it there.
Comment 4 Joseph Pecoraro 2010-02-13 10:25:41 PST
Note that this wouldn't handle a value like: "x-white", but no such case exists yet. The only existing tricky cases: "skip-white-space", "after-white-space", and "white-space" are handled.
Comment 5 Joseph Pecoraro 2010-02-27 23:10:16 PST
Landed in http://trac.webkit.org/changeset/55357
Committed r55357
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/StylesSidebarPane.js
r55357 = c977cf7ac5a4d5339a7e3ae93d290b350a8c4064 (trunk)