Currently accessor property is visible as 2 separate get- and set- tree elements. Gather them under a single node. This node will probably host a calculated value subnode too.
Created attachment 195194 [details] Patch
Comment on attachment 195194 [details] Patch Attachment 195194 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17327102 New failing tests: inspector/runtime/runtime-getProperties.html inspector/debugger/properties-special.html
Created attachment 195201 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Comment on attachment 195194 [details] Patch Attachment 195194 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17294291 New failing tests: inspector/runtime/runtime-getProperties.html inspector/debugger/properties-special.html
Created attachment 195203 [details] Archive of layout-test-results from gce-cr-linux-01 for chromium-linux-x86_64 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: chromium-linux-x86_64 Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment on attachment 195194 [details] Patch Attachment 195194 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17237594 New failing tests: inspector/runtime/runtime-getProperties.html inspector/debugger/properties-special.html
Created attachment 195321 [details] Archive of layout-test-results from webkit-ews-04 for mac-future The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-future Platform: Mac OS X 10.8.2
Created attachment 195447 [details] Patch
Comment on attachment 195447 [details] Patch Attachment 195447 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17324225 New failing tests: inspector/console/command-line-api.html
Created attachment 195457 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Comment on attachment 195447 [details] Patch Attachment 195447 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17257434 New failing tests: inspector/console/command-line-api.html
Created attachment 195460 [details] Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-03 Port: chromium-linux-x86_64 Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment on attachment 195447 [details] Patch Attachment 195447 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17216981 New failing tests: inspector/console/command-line-api.html
Created attachment 195464 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Comment on attachment 195447 [details] Patch Attachment 195447 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17344034 New failing tests: inspector/console/command-line-api.html
Created attachment 195466 [details] Archive of layout-test-results from gce-cr-linux-06 for chromium-linux-x86_64 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-06 Port: chromium-linux-x86_64 Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment on attachment 195447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195447&action=review Please attach screenshot. > Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:481 > + if (properties[i].value == null) == -> === > Source/WebCore/inspector/front-end/RemoteObject.js:218 > if (property.get || property.set) { style: no {} around one liners I'd rather move this get/set logic into constructor. > Source/WebCore/inspector/front-end/RemoteObject.js:453 > + if (descriptor && descriptor.get) { style: no {} around one liners > Source/WebCore/inspector/front-end/RemoteObject.js:456 > + if (descriptor && descriptor.set) { style: no {} around one liners > LayoutTests/inspector/debugger/properties-special.html:71 > + if (children[j].nameElement && children[j].nameElement.textContent === "__proto__") { style: no {} around one liners > LayoutTests/inspector/runtime/runtime-getProperties.html:63 > + function convertPropertyValueForTest(propertyObj, fieldName) propertyObj -> propertyObject
Created attachment 195575 [details] Patch
Created attachment 195576 [details] Screenshot
Created attachment 195577 [details] Screenshot 2
Comment on attachment 195575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195575&action=review > Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:621 > + markerElement.textContent = "\u2388"; Why HELM SYMBOL?
> Why HELM SYMBOL? I'm glad you someone asks this. I wanted a sing to distinguish accessor property from a normal one. The name is shown in italic, but I needed more explicit. The symbol was chosen randomly. Maybe some kind of gear symbol would be more relevant. Do you feel the helm is the wrong one? Do you think it would be wrong to commit (even temporary) such symbol into a repository?
(In reply to comment #22) > > Why HELM SYMBOL? > > I'm glad you someone asks this. I wanted a sing to distinguish accessor property from a normal one. The name is shown in italic, but I needed more explicit. > > The symbol was chosen randomly. Maybe some kind of gear symbol would be more relevant. > > Do you feel the helm is the wrong one? Do you think it would be wrong to commit (even temporary) such symbol into a repository? I don't think a symbol is needed, and italics is enough. But I'm not driving the design of the WebKit Web Inspector anymore either. It is up to you guys.
Created attachment 195669 [details] Patch
Comment on attachment 195447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195447&action=review >> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:481 >> + if (properties[i].value == null) > > == -> === Done >> Source/WebCore/inspector/front-end/RemoteObject.js:218 >> if (property.get || property.set) { > > style: no {} around one liners > I'd rather move this get/set logic into constructor. Done >> Source/WebCore/inspector/front-end/RemoteObject.js:453 >> + if (descriptor && descriptor.get) { > > style: no {} around one liners Done >> Source/WebCore/inspector/front-end/RemoteObject.js:456 >> + if (descriptor && descriptor.set) { > > style: no {} around one liners Done >> LayoutTests/inspector/debugger/properties-special.html:71 >> + if (children[j].nameElement && children[j].nameElement.textContent === "__proto__") { > > style: no {} around one liners Done >> LayoutTests/inspector/runtime/runtime-getProperties.html:63 >> + function convertPropertyValueForTest(propertyObj, fieldName) > > propertyObj -> propertyObject Done
Comment on attachment 195669 [details] Patch Clearing flags on attachment: 195669 Committed r147218: <http://trac.webkit.org/changeset/147218>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 113585
I have rolled this patch out. I don't think we have an agreement on the way we want to implement properties inspection in UI. I think property value should be accessible as a top level element. It could also have "Show getter definition" and "Show setter definition" items in context menu. In this case getter and setter could be kept inside prototype Object -- field: value -- * property: propertyValue -- __proto__ -- someMethod: function() ... -- get property: function() ... -- set property: function() ... If we move on with my suggestion we don't need this change at all, so we should decide first before landing this patch.
Also FYI: <mlam> vsevik: http://build.webkit.org/builders/Apple%20Lion%20Release%20WK2%20%28Tests%29/builds/9398 may have caused http://build.webkit.org/results/Apple%20Lion%20Release%20WK2%20(Tests)/r147218%20(9398)/inspector/console/console-format-table-pretty-diff.html