Summary: | DeprecatedGlobalSettings::globalConstRedeclarationShouldThrow() is not actually a setting, so should not be in DeprecatedGlobalSettings | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||
Component: | New Bugs | Assignee: | Sam Weinig <sam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=157101 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 250192 | ||||||||
Attachments: |
|
Description
Sam Weinig
2020-08-29 08:49:11 PDT
Created attachment 407539 [details]
Patch
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. (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. (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). 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. We promised to add a setting, in bug 157101, but looks like we never did. Which leads us to the appropriate bug in Radar! rdar://25965028 Created attachment 407582 [details]
Patch
Committed r266355: <https://trac.webkit.org/changeset/266355> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407582 [details]. |