Steps to Reproduce: 1. Open the Console 2. Type: document.body.style.col Results: I expected to see "color" autocomplete. Actual results were no autocompletions.
Created attachment 54905 [details] [PATCH] Proposed Fix The generated list in this example came from converting the properties list from the CSS Tokenizer. However, the list can be automatically generated like so: > // This is the list of properties > var properties = [] > var keywords = window.getComputedStyle(document.documentElement); > for (var i = 0, len = keywords.length; i < len; ++i) { > > // Strip leading hyphen from -vendor properties > var property = keywords[i]; > if (property.charAt(0) === "-") > property = property.substring(1); > > // Turn hypens points into camel case points > properties[i] = property.replace(/\-./g, function(c) { > return c.charAt(1).toUpperCase(); > }); > } > > // `properties` contains the results
I specifically wanted this so I could play with "webkitTransform" and friends =). I only tested use in the console. I'm not completely sure about other cases just yet. I just wanted some feedback on autogeneration Versus precompiled list before a final patch.
Comment on attachment 54905 [details] [PATCH] Proposed Fix WebCore/inspector/front-end/InjectedScript.js:133 + propertyNames = propertyNames.concat(InjectedScript._hiddenStyleProperties); Be careful, _getPropertyNames above already has style properties due to your changes to InjectedScript._getPropertyNames. In fact, if we are talking about autocomplete, you should only modify InjectedScript.getCompletions. Otherwise, style objects start containing weird properties in the object trees.
Good point, thanks for the review!
Created attachment 55779 [details] [PATCH] Improved Fix - Only happens for autocompletion. - Automatically generate the list of properties (I can convert this back if desired, but it is fast).
Comment on attachment 55779 [details] [PATCH] Improved Fix WebCore/inspector/front-end/InjectedScript.js:261 + var jsProperties = []; properties is a simplier name, js prefix isn't needed. Consider landing manually and not using CQ this time.
A small note, computed style does not have shorthands and is sometimes missing new properties, since they need to be added to the computed style list to show up.
(In reply to comment #7) > A small note, computed style does not have shorthands and is sometimes missing new properties, since they need to be added to the computed style list to show up. So would you prefer a pre-computed list, to be updated occasionally? Or maybe adding some kind of function that really does return a list of all style properties?
It would be nice to have a way to get all the properties (shorthands and all) from the inspector host.
We could use that for the syntax highlighter too.
(In reply to comment #9) > It would be nice to have a way to get all the properties > (shorthands and all) from the inspector host. (In reply to comment #10) > We could use that for the syntax highlighter too. Yes, that is what I meant. =) I will investigate this. This would really help keep in sync the following places: - console autocompletion (injected side) - styles autocompletion (sidebar) - css tokenizer (highlighting)
> > - console autocompletion (injected side) > - styles autocompletion (sidebar) > - css tokenizer (highlighting) Computed style is missing not only shorthands, but some longhand properties as well. I don't see border radius properties there for example. There is no single place with the CSS property names in the parser we could get them from, so whatever solution is, it would need to refactor some native code. As a short term solution, we could generate a single white list of properties and use it from within three subsystems you mentioned above. We could push the list from the front-end to the injected script lazily if necessary.
> As a short term solution, we could generate a single white list of > properties and use it from within three subsystems you mentioned > above. We could push the list from the front-end to the injected > script lazily if necessary. Should we include support for properties WebKit doesn't actually understand? Like other browser vendor properties. I don't think we should, that way we make it clear what is relevant in the browser running WebKit. I'm wondering if a backend solution could twist that into a good thing. For instance if the browser running the Web Inspector has doesn't support some style properties (say they are hidden behind an ENABLE flag) than that Web Inspector wouldn't highlight or autocomplete those properties. I'm just thinking out loud though. A pre-compiled / generated list is an easy solution, but I keep thinking its not easy to maintain. This is really something that I think should be investigated for a long term solution. (TAKE THAT Bugzilla! I can totally enforce my own 80 char wrap!)
> WebCore/inspector/front-end/InjectedScript.js:261 > + var jsProperties = []; > properties is a simplier name, js prefix isn't needed. Done. > I will investigate this. This would really help keep in sync > the following places: > > - console autocompletion (injected side) > - styles autocompletion (sidebar) > - css tokenizer (highlighting) Filed Bug for this: https://bugs.webkit.org/show_bug.cgi?id=40886 This would address Pavel's comments as well. For instance this patch didn't autocomplete "background" or "borderRadius" but it did longer versions. Committed r61506 M WebCore/ChangeLog M WebCore/inspector/front-end/InjectedScript.js r61506 = 72b23490dfc195079550fc7c3f17fdee30e8dd3a (refs/remotes/trunk) http://trac.webkit.org/changeset/61506
http://trac.webkit.org/changeset/61506 might have broken GTK Linux 32-bit Release, GTK Linux 64-bit Release, and Qt Linux Release
I think WebKit should just enumerate these properties, like all other browsers do. It will make this patch unnecessary. https://bugs.webkit.org/show_bug.cgi?id=23946
The following line causes the crash: var keywords = window.getComputedStyle(document.documentElement); Changing window to inspectedWindow, there is still the possibility that inspectedWindow.document is no longer defined (on navigation) and this causes the crash. Nikita bring up a good point. So, I am going to revert this, and try and tackle the problem from that angle.
Rolled out in r61507 r61507 = dc59d8744308cdc41b57e3aaf96b26f58e4fe4ef (refs/remotes/trunk) http://trac.webkit.org/changeset/61507 I'm going to take Nikita's suggestion!
I am not working on this for the time being. Someone else can jump on this.
Old bug, appears complete on Chrome.