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

Description Jinwoo Song 2014-03-23 21:47:10 PDT

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."

Comment 1 Ahmad Saleem 2024-03-25 05:59:37 PDT

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.

Comment 2 Ahmad Saleem 2024-04-03 21:58:28 PDT

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