RESOLVED FIXED 109426
Web Inspector: Simplify SplitView to rely more on CSS 
https://bugs.webkit.org/show_bug.cgi?id=109426
Summary Web Inspector: Simplify SplitView to rely more on CSS
Vladislav Kaznacheev
Reported 2013-02-11 06:51:11 PST
Currently WebInspector.SplitView code performs quite a few element.style manipulations. Most of these can replaced with simple CSS rules.
Attachments
Patch (16.34 KB, patch)
2013-02-11 06:54 PST, Vladislav Kaznacheev 		no flags
DetailsFormatted DiffDiff
Patch (15.40 KB, patch)
2013-02-12 05:34 PST, Vladislav Kaznacheev 		no flags
DetailsFormatted DiffDiff
Patch (15.43 KB, patch)
2013-02-13 04:14 PST, Vladislav Kaznacheev 		no flags
DetailsFormatted DiffDiff
Patch (4.85 KB, patch)
2013-02-14 07:52 PST, Vladislav Kaznacheev 		pfeldman: review-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Vladislav Kaznacheev
Comment 1 2013-02-11 06:54:35 PST
Created attachment 187561 [details] Patch
Alexander Pavlov (apavlov)
Comment 2 2013-02-11 07:05:04 PST
Comment on attachment 187561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187561&action=review Not really familiar with the code, but there's one style nit there. > Source/WebCore/inspector/front-end/SplitView.js:181 > + this._firstElement.style.cssText = ''; Web Inspector uses double quotes (") for enclosing string literals.
Vsevolod Vlasov
Comment 3 2013-02-12 05:14:12 PST
Comment on attachment 187561 [details] Patch I would revert the change in SplitView._removeAllLayoutProperties (what if split view elements have styles set externally?), other than that looks good.
Vladislav Kaznacheev
Comment 4 2013-02-12 05:34:04 PST
Comment on attachment 187561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187561&action=review >> Source/WebCore/inspector/front-end/SplitView.js:181 >> + this._firstElement.style.cssText = ''; > > Web Inspector uses double quotes (") for enclosing string literals. Vsevik convinced me to roll back the change to this function, so the problem is no longer there.
Vladislav Kaznacheev
Comment 5 2013-02-12 05:34:55 PST
Created attachment 187842 [details] Patch
WebKit Review Bot
Comment 6 2013-02-12 05:52:19 PST
Comment on attachment 187842 [details] Patch Rejecting attachment 187842 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-02', 'validate-changelog', '--non-interactive', 187842, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/16519216
Vladislav Kaznacheev
Comment 7 2013-02-13 04:14:37 PST
Created attachment 188051 [details] Patch
WebKit Review Bot
Comment 8 2013-02-13 05:17:04 PST
Comment on attachment 188051 [details] Patch Clearing flags on attachment: 188051 Committed r142738: <http://trac.webkit.org/changeset/142738>
WebKit Review Bot
Comment 9 2013-02-13 05:17:09 PST
All reviewed patches have been landed. Closing bug.
Andrey Adaikin
Comment 10 2013-02-14 07:29:02 PST
This patch broke SplitView in Canvas profiler. I requested a rollout, but sheriffbot failed. @Vladislav: Would you manually roll out this patch please, or plz fix the problem. A SplitView in CanvasProfileView.js no longer works.
Vladislav Kaznacheev
Comment 11 2013-02-14 07:52:57 PST
Created attachment 188351 [details] Patch
Pavel Feldman
Comment 12 2013-02-14 07:53:52 PST
Comment on attachment 188351 [details] Patch There is a bug-per patch policy that you should follow.
Andrey Adaikin
Comment 13 2013-02-14 08:16:33 PST
Regression addressed at https://bugs.webkit.org/show_bug.cgi?id=109835
Note You need to log in before you can comment on or make changes to this bug.