Bug 130658
| Summary: | Recompute gapLen in deleteInsignificantText() | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> |
| Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | ahmad.saleem792, rniwa, wenson_hsieh |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Jinwoo Song
According to Darin's comment in bug 130634, we should verify if the code is incorrect and the gapLen should be recomputed.
"I suspect the dead store here indicates a bug. I think that gapLen needs to be recomputed. Instead of removing this line of code, we should add a line of code: gapLen = gapEnd - gapStart; But of course, we also need to construct a test case to demonstrate that the code is incorrect, which could be difficult."
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Ahmad Saleem
NOTE - It is about this code:
https://searchfox.org/wubkat/rev/de98fcee68c0bd94f16d6dfb9dac2ea106ee40d5/Source/WebCore/editing/CompositeEditCommand.cpp#1070
gapEnd = std::min(gapEnd, end);
should be replaced with
gapLength = gapEnd - gapStart;
* gapLen was renamed to 'gapLength' over the period of time.
___
It compiles with above line change using `build-webkit --release`.
From imported WPT 'editing' tests - I didn't find any regression. As for our own local tests - there were few 'diff' but I had other patches on local (related to HTML Editing), so I couldn't narrow it down which are caused by this change.
Just sharing up-to-date status.
Ahmad Saleem
Hmm.. Came across following from Blink's commit that 'static analyzer' complained on 'gapLen', so it was removed completely:
https://src.chromium.org/viewvc/blink/trunk/Source/core/editing/CompositeEditCommand.cpp?r1=164628&r2=164627&pathrev=164628
Commit: https://src.chromium.org/viewvc/blink?view=revision&revision=164628