Bug 147109

Summary: Web Inspector: Update to CodeMirror 5.5 or later
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: 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 Flags
Patch
none
Patch
none
[Animated GIF] Before/after the patch
none
Patch none

Timothy Hatcher
Reported 2015-07-20 07:39:24 PDT
CodeMirror 5.5 is out. We should update to the latest. https://twitter.com/codemirror/status/623085362022359040
Attachments
Patch (314.74 KB, patch)
2015-08-11 10:07 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (325.07 KB, patch)
2015-08-11 16:34 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
[Animated GIF] Before/after the patch (43.29 KB, image/gif)
2015-08-11 17:18 PDT, Nikita Vasilyev 		no flags
Details
Patch (326.67 KB, patch)
2015-08-11 21:45 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-07-20 07:39:32 PDT
<rdar://problem/21898870>
Timothy Hatcher
Comment 2 2015-07-20 07:43:36 PDT
We are using 4.2.0 currently.
Devin Rousso
Comment 3 2015-08-11 10:07:22 PDT
Created attachment 258723 [details] Patch
Timothy Hatcher
Comment 4 2015-08-11 10:45:30 PDT
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.
Timothy Hatcher
Comment 5 2015-08-11 10:45:49 PDT
Please make sure to test by running OpenSource/Source/WebInspectorUI/Scripts/copy-user-interface-resources-dryrun.rb
Devin Rousso
Comment 6 2015-08-11 13:08:20 PDT
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.
Devin Rousso
Comment 7 2015-08-11 13:16:40 PDT
(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?
Joseph Pecoraro
Comment 8 2015-08-11 13:38:42 PDT
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
Devin Rousso
Comment 9 2015-08-11 16:34:37 PDT
Created attachment 258779 [details] Patch
Nikita Vasilyev
Comment 10 2015-08-11 17:18:54 PDT
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.
Timothy Hatcher
Comment 11 2015-08-11 19:28:53 PDT
Yes, we need to fix those things. It can happen in a follow up, as long as we don't forget.
Devin Rousso
Comment 12 2015-08-11 21:45:18 PDT
Created attachment 258802 [details] Patch
WebKit Commit Bot
Comment 13 2015-08-11 22:40:49 PDT
Comment on attachment 258802 [details] Patch Clearing flags on attachment: 258802 Committed r188325: <http://trac.webkit.org/changeset/188325>
WebKit Commit Bot
Comment 14 2015-08-11 22:40:52 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.