Bug 215978 – DeprecatedGlobalSettings::globalConstRedeclarationShouldThrow() is not actually a setting, so should not be in DeprecatedGlobalSettings

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

DeprecatedGlobalSettings::globalConstRedeclarationShouldThrow() is not actually a setting, so should not be in DeprecatedGlobalSettings

[Sam Weinig]

Created attachment 407539 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 407539 [details]
Patch

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

> Source/WebCore/bindings/js/CommonVM.cpp:47
> +static bool globalConstRedeclarationShouldThrow()

I  know that you are only moving code, but this needs a comment with a radar that tracks when this can be removed. Perhaps it can already be removed.

[Sam Weinig]

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 407539 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407539&action=review
> 
> > Source/WebCore/bindings/js/CommonVM.cpp:47
> > +static bool globalConstRedeclarationShouldThrow()
> 
> I  know that you are only moving code, but this needs a comment with a radar
> that tracks when this can be removed. Perhaps it can already be removed.

Sure, feel free to file one and add the comment. I don't think this change should be predicated on that though.

[Sam Weinig]

(In reply to Sam Weinig from comment #3)
> (In reply to Alexey Proskuryakov from comment #2)
> > Comment on attachment 407539 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=407539&action=review
> > 
> > > Source/WebCore/bindings/js/CommonVM.cpp:47
> > > +static bool globalConstRedeclarationShouldThrow()
> > 
> > I  know that you are only moving code, but this needs a comment with a radar
> > that tracks when this can be removed. Perhaps it can already be removed.
> 
> Sure, feel free to file one and add the comment. I don't think this change
> should be predicated on that though.

That is to say, I'm not exactly sure what radar you want, so if you file one, I'd be happy to add a FIXME in this change (or later).

[Darin Adler]

Comment on attachment 407539 [details]
Patch

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

>>>> Source/WebCore/bindings/js/CommonVM.cpp:47
>>>> +static bool globalConstRedeclarationShouldThrow()
>>> 
>>> I  know that you are only moving code, but this needs a comment with a radar that tracks when this can be removed. Perhaps it can already be removed.
>> 
>> Sure, feel free to file one and add the comment. I don't think this change should be predicated on that though.
> 
> That is to say, I'm not exactly sure what radar you want, so if you file one, I'd be happy to add a FIXME in this change (or later).

Saam, when you landed this in r200121, 4 years ago, was there a bug in Radar to track work in iBooks to allow this to be removed? Without a setting, I wonder if they can ever remove it, since they want compatibility with old books.

[Darin Adler]

We promised to add a setting, in bug 157101, but looks like we never did.

[Darin Adler]

Which leads us to the appropriate bug in Radar!

rdar://25965028

[Sam Weinig]

Created attachment 407582 [details]
Patch

[EWS]

Committed r266355: <https://trac.webkit.org/changeset/266355>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407582 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68079901>