Summary: | Web Inspector: Refactorings: Prepare SuggestBox for reuse. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eugene Klyuchnikov <eustas> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Eugene Klyuchnikov <eustas> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 111427 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Eugene Klyuchnikov
2013-03-04 04:24:21 PST
Created attachment 191203 [details]
Patch
Comment on attachment 191203 [details] Patch 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? Comment on attachment 191203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191203&action=review >> Source/WebCore/inspector/front-end/SuggestBox.js:60 >> + this._anchorElement = anchorElement; > > This field is unused, hence may be removed altogether. _anchorElement is now used in _updateBoxPosition >> Source/WebCore/inspector/front-end/SuggestBox.js:98 >> + * @param {AnchorBox=} anchorBox > > Any cases where the anchorBox is not supplied to _updateBoxPosition()? My next patch uses SuggestionBox this way. But I think you are right - this change could be separate from refactoring. >> Source/WebCore/inspector/front-end/SuggestBox.js:330 >> + if (this._canShowBox(completions, !!canShowForSingleItem, userEnteredText)) { > > Why "!!canShowForSingleItem" if it is mandatory? oops. That was mandatory for previous signature. I'll revert this change. Created attachment 191490 [details]
Patch
Comment on attachment 191490 [details] Patch Clearing flags on attachment: 191490 Committed r145054: <http://trac.webkit.org/changeset/145054> All reviewed patches have been landed. Closing bug. |