Bug 111191 - Web Inspector: add Ace editor experiment
Summary: Web Inspector: add Ace editor experiment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Lushnikov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-01 09:49 PST by Andrey Lushnikov
Modified: 2013-03-15 05:52 PDT (History)
10 users (show)

See Also:

Attachments
Patch (684.66 KB, patch)
2013-03-01 10:00 PST, Andrey Lushnikov 		no flags Details | Formatted Diff | Diff
Patch (682.04 KB, patch)
2013-03-01 10:04 PST, Andrey Lushnikov 		no flags Details | Formatted Diff | Diff
Patch (682.17 KB, patch)
2013-03-01 10:39 PST, Andrey Lushnikov 		no flags Details | Formatted Diff | Diff
Patch (682.05 KB, patch)
2013-03-04 01:29 PST, Andrey Lushnikov 		no flags Details | Formatted Diff | Diff
Patch (683.89 KB, patch)
2013-03-04 01:48 PST, Andrey Lushnikov 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Lushnikov 2013-03-01 09:49:05 PST 
Add ace editor experiment.
Comment 1 Andrey Lushnikov 2013-03-01 10:00:32 PST 
Created attachment 190981 [details]
Patch
Comment 2 Andrey Lushnikov 2013-03-01 10:04:03 PST 
Created attachment 190982 [details]
Patch
Comment 3 Pavel Feldman 2013-03-01 10:12:26 PST 
Comment on attachment 190982 [details]
Patch

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

Could you share the stats on how much bigger the front-end footprint will be? In absolute numbers as well as percentage. Raw, minified and zipped (chromium resources are zipped).

> Source/WebCore/inspector/front-end/AceTextEditor.js:48
> +    this.element.style.height = "100%";

Please use CSS for styling.

> Source/WebCore/inspector/front-end/AceTextEditor.js:84
> +            case "text/html": this._aceEditor.getSession().setMode("ace/mode/html"); break;

Do not indent case from switch.
Comment 4 Andrey Lushnikov 2013-03-01 10:39:10 PST 
Created attachment 190985 [details]
Patch
Comment 5 Andrey Lushnikov 2013-03-04 01:25:43 PST 
Some stats on Ace disk usage:
1. Raw folder size of Ace files in this patch is 664Kb
2. If I were to put minified version of ace files, it would be 376Kb

I estimated gzip size of devtools_resources.pak file. 
Without this patch: 824Kb
With this patch: 956Kb
So it brings in 132Kb of overhead.
Comment 6 Andrey Lushnikov 2013-03-04 01:29:56 PST 
Created attachment 191174 [details]
Patch
Comment 7 Pavel Feldman 2013-03-04 01:37:27 PST 
Comment on attachment 191174 [details]
Patch

Please make sure you include original BSD LICENSE file.
Comment 8 Andrey Lushnikov 2013-03-04 01:48:54 PST 
Created attachment 191176 [details]
Patch
Comment 9 WebKit Review Bot 2013-03-04 03:47:51 PST 
Comment on attachment 191176 [details]
Patch

Clearing flags on attachment: 191176

Committed r144615: <http://trac.webkit.org/changeset/144615>
Comment 10 WebKit Review Bot 2013-03-04 03:47:54 PST 
All reviewed patches have been landed.  Closing bug.
Comment 11 Fabian Jakobs 2013-03-14 01:59:29 PDT 
Hi, I'm the author of ace and very excited to see it landing in Chrome. I have noticed a few glitches with this patch:

- the breakpoint context menu does not work
- I don't get syntax highlighting for css files

If you prefer I can create separate issues for this.
Comment 12 Andrey Lushnikov 2013-03-15 05:52:14 PDT 
Hi Fabian,

First of all, thank you for your great project!

We are aware of the bugs you've mentioned. We've added Ace editor as an experiment and it's not fully embedded into the devtools (that's the reason for the glitches you've mentioned). There's no decision yet if we would invest efforts in its adoption and use it as our primary code editor.