Bug 159991 - Kill legacy valueToStringWithNullCheck() utility function
Summary: Kill legacy valueToStringWithNullCheck() utility function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-20 16:19 PDT by Chris Dumez
Modified: 2016-07-21 11:41 PDT (History)
12 users (show)

See Also:


Attachments
Patch (4.30 KB, patch)
2016-07-20 16:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-07-20 16:19:01 PDT
Kill legacy valueToStringWithNullCheck() utility function. Treating null as a null string is legacy behavior so drop this function so that people are not tempted to use it. We should be using either:
1. JSValue::toWTFString() for non-nullable DOMStrings
2. valueToStringWithUndefinedOrNullCheck() for nullable DOMStrings
3. valueToStringTreatingNullAsEmptyString() for strings with [TreatNullAs=EmptyString]
Comment 1 Chris Dumez 2016-07-20 16:26:46 PDT
Created attachment 284165 [details]
Patch
Comment 2 Chris Dumez 2016-07-21 09:02:06 PDT
Comment on attachment 284165 [details]
Patch

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

> Source/WebCore/bindings/js/JSHTMLFrameElementCustom.cpp:57
> +    String locationValue = value.isNull() ? String() : value.toWTFString(&state);

I maintained the previous behavior for this setter (treating null as the null String) even though this behavior is non-standard since the attribute is non-standard. It was never implemented in Firefox / IE and Chrome dropped it a while back after getting usage data showing this was barely used.
I personally think we should try and drop this attribute as well but I did not want to do this in this patch.
Comment 3 Chris Dumez 2016-07-21 11:40:59 PDT
Comment on attachment 284165 [details]
Patch

Clearing flags on attachment: 284165

Committed r203516: <http://trac.webkit.org/changeset/203516>
Comment 4 Chris Dumez 2016-07-21 11:41:04 PDT
All reviewed patches have been landed.  Closing bug.