Add allowJavaScriptFromSource callback to FrameLoaderClient
Created attachment 112674 [details] Patch
Hey Adam, can you have a look please?
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 112674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112674&action=review > Source/WebCore/loader/FrameLoaderClient.h:302 > virtual bool allowJavaScript(bool enabledPerSettings) { return enabledPerSettings; } perhaps these methods should really be allowScript and allowScriptFromSource. we normally leave off the "java" prefix in front of script.
Comment on attachment 112674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112674&action=review > Source/WebCore/loader/FrameLoaderClient.h:303 > + virtual bool allowJavaScriptFromSource(bool enabledPerSettings, const KURL&) { return enabledPerSettings; } At least for allowJavaScriptFromSource, it should be allowScriptFromSource. I think it's fine to change allowJavaScript in a follow-up patch. (That's mostly a search-and-replace job.)
(In reply to comment #5) > (From update of attachment 112674 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112674&action=review > > > Source/WebCore/loader/FrameLoaderClient.h:303 > > + virtual bool allowJavaScriptFromSource(bool enabledPerSettings, const KURL&) { return enabledPerSettings; } > > At least for allowJavaScriptFromSource, it should be allowScriptFromSource. I think it's fine to change allowJavaScript in a follow-up patch. (That's mostly a search-and-replace job.) Basically, renaming allowJavaScript to allowScript is easy. However, there's still isJavaScriptEnabled(), and renaming that touches lots of ports. not sure how happy they would be about a setting being renamed
They'll be fine as long as you don't change their public API. In any case, doing this in a bunch of small patches seems best.
Created attachment 112721 [details] Patch
Comment on attachment 112721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112721&action=review LGTM, but we need fishd to do the API review. > Tools/DumpRenderTree/chromium/WebPermissions.h:62 > - // Resets the policy to allow images, storage, displaying insecure, but not running insecure. > + // Resets the policy to allow images, scripts, storage, displaying insecure, but not running insecure. I'd just remove this comment or generalize it so we don't need to edit it all the time.
Created attachment 112820 [details] Patch
Comment on attachment 112820 [details] Patch Clearing flags on attachment: 112820 Committed r98708: <http://trac.webkit.org/changeset/98708>
All reviewed patches have been landed. Closing bug.