[webkit-reviews] review denied: [Bug 111301] Web Inspector: Refactorings: Prepare SuggestBox for reuse. : [Attachment 191203] Patch

[bugzilla-daemon at webkit.org]

Alexander Pavlov (apavlov) <apavlov at chromium.org> has denied Eugene Klyuchnikov
<eustas at chromium.org>'s request for review:
Bug 111301: Web Inspector: Refactorings: Prepare SuggestBox for reuse.
https://bugs.webkit.org/show_bug.cgi?id=111301

Attachment 191203: Patch
https://bugs.webkit.org/attachment.cgi?id=191203&action=review

------- Additional Comments from Alexander Pavlov (apavlov)
<apavlov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191203&action=review


It is quite hard to track the changes in this patch even for the author of this
code. Let us separate the patch into a few others, and not land provisional
code (you can make the respective changes in the patch that uses the new code.)


> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2649
> + * @param {?WebInspector.CSSMetadata} cssCompletions

Why is it nullable???

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:-2714
> -	   if (!word)

Unnecessary change.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2730
> +	   var cssCompletions = this._cssCompletions;

Ditto.

> Source/WebCore/inspector/front-end/SuggestBox.js:60
> +    this._anchorElement = anchorElement;

This field is unused, hence may be removed altogether.

> Source/WebCore/inspector/front-end/SuggestBox.js:98
> +	* @param {AnchorBox=} anchorBox

Any cases where the anchorBox is not supplied to _updateBoxPosition()?

> Source/WebCore/inspector/front-end/SuggestBox.js:330
> +	   if (this._canShowBox(completions, !!canShowForSingleItem,
userEnteredText)) {

Why "!!canShowForSingleItem" if it is mandatory?