Bug 89818 - Inspector crashes when trying to inspect a page with CSS variables
Summary: Inspector crashes when trying to inspect a page with CSS variables
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Luke Macpherson
URL: http://trac.webkit.org/export/HEAD/tr...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-23 15:02 PDT by Dimitri Glazkov (Google)
Modified: 2012-07-04 17:16 PDT (History)
12 users (show)

See Also:


Attachments
Patch (5.67 KB, patch)
2012-06-25 18:04 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff
Patch (4.89 KB, patch)
2012-06-25 20:56 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff
Patch (7.97 KB, patch)
2012-06-28 18:37 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff
Patch (9.83 KB, patch)
2012-06-28 18:50 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (748.88 KB, application/zip)
2012-06-28 22:02 PDT, WebKit Review Bot
no flags Details
Patch (13.73 KB, patch)
2012-07-01 18:41 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 (336.14 KB, application/zip)
2012-07-02 05:36 PDT, WebKit Review Bot
no flags Details
Same Patch - retry on bots. (13.70 KB, patch)
2012-07-02 16:51 PDT, Luke Macpherson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2012-06-23 15:02:54 PDT
1) Go to URL above
2) Try to inspect the element that has CSS Variables applied to it
3) ... boom.
Comment 1 Luke Macpherson 2012-06-24 16:31:56 PDT
Are you using a custom build with --css-variables to reproduce this?
Comment 2 Dimitri Glazkov (Google) 2012-06-24 16:36:39 PDT
(In reply to comment #1)
> Are you using a custom build with --css-variables to reproduce this?

Yup.
Comment 3 Luke Macpherson 2012-06-25 18:04:53 PDT
Created attachment 149414 [details]
Patch
Comment 4 Dimitri Glazkov (Google) 2012-06-25 18:28:30 PDT
Comment on attachment 149414 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:2974
> +    printf("storing variable name \"%s\"\n", tmp.ascii().data());

printf :)
Comment 5 Dimitri Glazkov (Google) 2012-06-25 18:28:51 PDT
Pavel, can you help Luke with the test for this?
Comment 6 Luke Macpherson 2012-06-25 20:56:00 PDT
Created attachment 149441 [details]
Patch
Comment 7 Pavel Feldman 2012-06-25 23:37:29 PDT
(In reply to comment #5)
> Pavel, can you help Luke with the test for this?

There is a bunch of these under LayoutTests/inspector/styles. You'll probably want to land them disabled until your feature is enabled (oh how do we handle these cases?). Adding apavlov@ for additional help.
Comment 8 Luke Macpherson 2012-06-28 18:37:57 PDT
Created attachment 150061 [details]
Patch
Comment 9 Luke Macpherson 2012-06-28 18:50:27 PDT
Created attachment 150063 [details]
Patch
Comment 10 Luke Macpherson 2012-06-28 18:52:03 PDT
Last patch adds a test to inspector, but we still need to disable that test if CSS_VARIABLES compile flag is off. One option would be to just move that test under fast/css/variables, where it will be ignored unless the flag is on.
Comment 11 WebKit Review Bot 2012-06-28 22:02:19 PDT
Comment on attachment 150063 [details]
Patch

Attachment 150063 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13119219

New failing tests:
inspector/styles/css-variables.html
Comment 12 WebKit Review Bot 2012-06-28 22:02:25 PDT
Created attachment 150083 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 13 Alexander Pavlov (apavlov) 2012-06-29 01:10:43 PDT
(In reply to comment #10)
> Last patch adds a test to inspector, but we still need to disable that test if CSS_VARIABLES compile flag is off. One option would be to just move that test under fast/css/variables, where it will be ignored unless the flag is on.

Since inspector tests require special setup, and they are filtered by the "/inspector" file URL part, I believe it's easier to add a directory like "LayoutTests/inspector/variables" to the result of _missing_symbol_to_skipped_tests() for the "CSSVariableValue" key (in Tools/Scripts/webkitpy/layout_tests/port/webkit.py) and get the benefit of the web inspector tests run only with the css variables enabled.
Comment 14 Luke Macpherson 2012-07-01 18:41:59 PDT
Created attachment 150353 [details]
Patch
Comment 15 WebKit Review Bot 2012-07-02 05:36:16 PDT
Comment on attachment 150353 [details]
Patch

Attachment 150353 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13133092

New failing tests:
fast/loader/loadInProgress.html
Comment 16 WebKit Review Bot 2012-07-02 05:36:21 PDT
Created attachment 150403 [details]
Archive of layout-test-results from gce-cr-linux-08

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 17 Luke Macpherson 2012-07-02 16:51:59 PDT
Created attachment 150498 [details]
Same Patch - retry on bots.
Comment 18 Luke Macpherson 2012-07-03 17:24:01 PDT
I can haz r+?
Comment 19 WebKit Review Bot 2012-07-04 17:16:14 PDT
Comment on attachment 150498 [details]
Same Patch - retry on bots.

Clearing flags on attachment: 150498

Committed r121874: <http://trac.webkit.org/changeset/121874>
Comment 20 WebKit Review Bot 2012-07-04 17:16:20 PDT
All reviewed patches have been landed.  Closing bug.