Bug 109426

Summary:

Web Inspector: Simplify SplitView to rely more on CSS

Product:

WebKit

Reporter:

Vladislav Kaznacheev <kaznacheev>

Component:

Web Inspector (Deprecated)

Assignee:

Vladislav Kaznacheev <kaznacheev>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aandrey, apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	pfeldman: review-

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	Details Formatted Diff Diff
Patch (15.40 KB, patch) 2013-02-12 05:34 PST, Vladislav Kaznacheev	no flags	Details Formatted Diff Diff
Patch (15.43 KB, patch) 2013-02-13 04:14 PST, Vladislav Kaznacheev	no flags	Details Formatted Diff Diff
Patch (4.85 KB, patch) 2013-02-14 07:52 PST, Vladislav Kaznacheev	pfeldman: review-	Details Formatted Diff Diff
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.