Bug 51114 – Web Inspector: Up/Down/PageUp/PageDown on a CSS property numeric value commit the value editor

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Alexander Pavlov (apavlov)]

[Page]Up/Down should not commit a CSS property value altogether but instead apply it to the style and continue editing.

Patch to follow

[Alexander Pavlov (apavlov)]

Created attachment 76660 [details]
[PATCH] Suggested fix

[Yury Semikhatsky]

Comment on attachment 76660 [details]
[PATCH] Suggested fix

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

> WebCore/inspector/front-end/ElementsPanel.js:-89
> -    this.sidebarPanes.styles.addEventListener("style property toggled", this._stylesPaneEdited, this);

_stylesPaneEdited is now only called from _styleSheetChanged, please get rid of _stylesPaneEdited

[Alexander Pavlov (apavlov)]

Created attachment 76665 [details]
[PATCH] Fix reimplemented correctly

pfeldman@ has explained the idea behind the event handling. This patch reverts the erroneous fix from r74038 and augments the stylesheet onRevert handling

[Eric Seidel (no email)]

Comment on attachment 76665 [details]
[PATCH] Fix reimplemented correctly

How do we test this?

[Alexander Pavlov (apavlov)]

Created attachment 76751 [details]
[PATCH] Test added

[Pavel Feldman]

(In reply to comment #4)
> (From update of attachment 76665 [details])
> How do we test this?

Eric, could you please not blindly r- inspector changes that are missing tests without meaningful review comments? They go away from my to review report and this slows down the development.

If you are willing to help us with the testing, as you know, inspector tests became flaky at some point and something in the harness needs to be fixed. I already had similar discussion with Adam the other day.

[Alexander Pavlov (apavlov)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       LayoutTests/ChangeLog
        M       LayoutTests/inspector/styles-add-blank-property-expected.txt
        M       LayoutTests/inspector/styles-add-blank-property.html
        M       WebCore/ChangeLog
        M       WebCore/inspector/front-end/ElementsPanel.js
Committed r74185

[WebKit Review Bot]

http://trac.webkit.org/changeset/74185 might have broken Leopard Intel Debug (Tests)