Bug 89847 – Change the serialization format of form control state to make the code simple

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kent Tamura]

https://bugs.webkit.org/show_bug.cgi?id=89409#c6
> The state (de)serialization flow looks becoming a bit cryptic. 
> My feeling is that it is because the state is stored as a string vector instead of string.
> If we control the format, the code will become much cleaner.

[Kent Tamura]

Created attachment 149229 [details]
Patch

[Kent Tamura]

Created attachment 149230 [details]
Patch 2

Remove TAB

[Hajime Morrita]

Comment on attachment 149230 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=149230&action=review

> Source/WebCore/html/FormController.cpp:61
> +        }

Don't we have any escape function? If so, let's use it. Or let's define one.

> Source/WebCore/html/FormController.cpp:95
> +    }

Ditto.

[Kent Tamura]

Comment on attachment 149230 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=149230&action=review

>> Source/WebCore/html/FormController.cpp:61
>> +        }
> 
> Don't we have any escape function? If so, let's use it. Or let's define one.

I couldn't find a generic escaping function. It's nice to have one because we have a similar code in CalendarPickerElement.cpp.

>> Source/WebCore/html/FormController.cpp:95
>> +    }
> 
> Ditto.

I have no good idea of unescaping function interface because we can't find delimiters of escaped strings without parsing.

[Kent Tamura]

Created attachment 149246 [details]
Patch 3

Add an escape function

[Kent Tamura]

Created attachment 149248 [details]
Patch 4

[WebKit Review Bot]

Comment on attachment 149248 [details]
Patch 4

Clearing flags on attachment: 149248

Committed r121145: <http://trac.webkit.org/changeset/121145>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

This seems like a step backwards to me. Serializing and then decoding the object is extra work. There is more code here than before.

[Kent Tamura]

(In reply to comment #9)
> This seems like a step backwards to me. Serializing and then decoding the object is extra work. There is more code here than before.

ok, I'll roll it out.  I don't have strong preference for this.

[Kent Tamura]

Reverted r121145 for reason:

Had an objection for the change.

Committed r121207: <http://trac.webkit.org/changeset/121207>

[Darin Adler]

Maybe I came on too strong.

I liked the new helper function for appending an escaped string.

It’s also good to have all the state wrapped up in a single object rather than a confusing combination of a vector and a size.

But I do want us to keep serializing and then re-parsing to a minimum.

[Levi Weintraub]

Reopening to attach new patch.

[Levi Weintraub]

Created attachment 157818 [details]
Patch

[Levi Weintraub]

Comment on attachment 157818 [details]
Patch

Whoops!! Wrong bug number! Sorry for the noise.

[Levi Weintraub]

Reopening to attach new patch.