| Summary: | Web Inspector: fix highlight bug in DTE. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andrey Lushnikov <lushnikov> | ||||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Andrey Lushnikov <lushnikov> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Andrey Lushnikov
2013-01-21 02:16:05 PST
Created attachment 183740 [details]
broken highlight screenshot
Created attachment 183742 [details]
Patch
Comment on attachment 183742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183742&action=review > Source/WebCore/ChangeLog:8 > + Fix highlight bug related to highlightChunkLimit in DefaultTextEditor. You should explain what has changed and why > Source/WebCore/inspector/front-end/TextEditorHighlighter.js:188 > + } else { What if there is no midConditionStringified? Will you hit NPE at state.ranges.push? no {} around single line blocks. Why creating ranges early Comment on attachment 183742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183742&action=review >> Source/WebCore/inspector/front-end/TextEditorHighlighter.js:188 >> + } else { > > What if there is no midConditionStringified? Will you hit NPE at state.ranges.push? > > no {} around single line blocks. Why creating ranges early Let's go through the following cases: 1. midConditionStringified and postConditionStringified both do not exist In this case, we're going to highlight the line from the scratch, and in this case we will create an empty state.ranges array. 2. postConditionStringified does not exist, but midConditionStringified exists. In this case, we don't want to re-create that array, because it contains highlight information. What is important is that the first case will always be hit before the second case (due to code logic). Addressing the NPE threat you've mentioned, it might be more robust to write smth like "state.ranges = state.ranges || []"; Comment on attachment 183742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183742&action=review >>> Source/WebCore/inspector/front-end/TextEditorHighlighter.js:188 >>> + } else { >> >> What if there is no midConditionStringified? Will you hit NPE at state.ranges.push? >> >> no {} around single line blocks. Why creating ranges early > > Let's go through the following cases: > 1. midConditionStringified and postConditionStringified both do not exist > In this case, we're going to highlight the line from the scratch, and in this case we will create an empty state.ranges > array. > > 2. postConditionStringified does not exist, but midConditionStringified exists. > In this case, we don't want to re-create that array, because it contains highlight information. > > What is important is that the first case will always be hit before the second case (due to code logic). > > Addressing the NPE threat you've mentioned, it might be more robust to write smth like "state.ranges = state.ranges || []"; >> state.ranges = state.ranges || [] Yep, I would have no problems understanding this one! Created attachment 183751 [details]
Patch
Comment on attachment 183751 [details] Patch Clearing flags on attachment: 183751 Committed r140320: <http://trac.webkit.org/changeset/140320> All reviewed patches have been landed. Closing bug. |