| Summary: | Web Inspector: Update to CodeMirror 5.5 or later | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||||||
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | commit-queue, graouts, hi, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 146115 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Timothy Hatcher
2015-07-20 07:39:24 PDT
We are using 4.2.0 currently. Created attachment 258723 [details]
Patch
Comment on attachment 258723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258723&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:444 > - if (this._codeMirror.options.readOnly || this._codeMirror.state.focused) > + if (this._codeMirror.options.readOnly) No way to get focused state now? > Source/WebInspectorUI/UserInterface/Views/HoverMenu.css:62 > + -webkit-transform: translateY(-1px); Why? No need to prefix now. > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.css:184 > - -webkit-transform: translateY(1px); > + -webkit-transform: translateY(0.5px); This is curious. Why? No need to prefix now. Please make sure to test by running OpenSource/Source/WebInspectorUI/Scripts/copy-user-interface-resources-dryrun.rb Comment on attachment 258723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258723&action=review >> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:444 >> + if (this._codeMirror.options.readOnly) > > No way to get focused state now? I think you can still get the focus in the same way, but it seems to fire before this event. Also, I think that it should still insert newlines even after the editor is focused. >> Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.css:184 >> + -webkit-transform: translateY(0.5px); > > This is curious. Why? No need to prefix now. This was purely a style change. It appeared slightly offset to me. (In reply to comment #5) > Please make sure to test by running > OpenSource/Source/WebInspectorUI/Scripts/copy-user-interface-resources- > dryrun.rb It's asking for a destination directory. What exactly does this do/how do I use it? Comment on attachment 258723 [details]
Patch
Make sure to test the Pretty Printer as well.
1. Open Source/WebInspectorUI/Tools/PrettyPrinter/index.html
2. Run the tests for each of the different "modes" you should see all PASS messages
Created attachment 258779 [details]
Patch
Created attachment 258784 [details]
[Animated GIF] Before/after the patch
There are a couple of issues with the patch:
— Increased line-height.
— vertically misaligned in-code elements, such as go-to arrow icons and type profiler tokens.
I can fix them as a follow up.
Yes, we need to fix those things. It can happen in a follow up, as long as we don't forget. Created attachment 258802 [details]
Patch
Comment on attachment 258802 [details] Patch Clearing flags on attachment: 258802 Committed r188325: <http://trac.webkit.org/changeset/188325> All reviewed patches have been landed. Closing bug. |