Bug 193439 - [GTK][WPE] Add enable-javascript-markup setting
Summary: [GTK][WPE] Add enable-javascript-markup setting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2019-01-15 01:17 PST by Carlos Garcia Campos
Modified: 2021-10-12 15:36 PDT (History)
10 users (show)

See Also:


Attachments
Patch (11.09 KB, patch)
2019-01-15 01:19 PST, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2019-01-15 01:17:04 PST
To disable js markup instead of all js execution.
Comment 1 Carlos Garcia Campos 2019-01-15 01:19:27 PST
Created attachment 359147 [details]
Patch
Comment 2 EWS Watchlist 2019-01-15 01:22:34 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Adrian Perez 2019-01-15 03:37:55 PST
Comment on attachment 359147 [details]
Patch

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

Informally reviewing, this would be a r- for my: the description
in the API documentation is seriously lacking, and the name of the
setting is quite bad (non-descriptive, and difficult to search
online for). I don't care what WebKit calls the setting internally,
I would rather have a name for it that better indicates what it does
*and* that we provide a good description of what the setting does and
its intended usage.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.

What is “JavaScript Markup”? A specification, a WebKit specific concept?
Something widely understood among Web developers?... I think the documentation
for this setting needs an explanation of what the option does, as it is nearly
impossible to find anything about this topic that would seem relevant when
implementing an application which makes use of WebKit.

After some digging in the repository history I arrived at bug #112999 and
bug #113122 and it took me wading through a pile of comments and reading
some bits of the code to guess what the setting does. So please let's make
the API documentation better by adding something in the lines of:

  “Enabling this setting will strip <script> tags from loaded HTML
   content, but other forms of JavaScript execution e.g. using
   webkit_web_view_run_javascript() are still allowed. This setting
   is intended for applications which display HTML content but are
   not full browsers, and which want to avoid the risk of script
   injection attacks, as is the case of applications like e-mail
   and news readers.”

If possible it should be more concrete that the above explaining what
gets restricted and what not, because very often we programmers end up
introducing accidental security vulnerabilities due to assumptions we
make caused by incomplete documentation of third party code used.
Comment 4 Adrian Perez 2019-01-15 03:44:16 PST
FWIW, my main issue with this is the lack of explanation in
the API documentation. The name is quite “meh”, but that is
less of a concern if the others out there prefer to keep it
as is :)
Comment 5 Carlos Garcia Campos 2019-01-15 04:19:13 PST
(In reply to Adrian Perez from comment #3)
> Comment on attachment 359147 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359147&action=review
> 
> Informally reviewing, this would be a r- for my: the description
> in the API documentation is seriously lacking, and the name of the
> setting is quite bad (non-descriptive, and difficult to search
> online for). I don't care what WebKit calls the setting internally,
> I would rather have a name for it that better indicates what it does
> *and* that we provide a good description of what the setting does and
> its intended usage.
> 
> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> > + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
> 
> What is “JavaScript Markup”? A specification, a WebKit specific concept?
> Something widely understood among Web developers?... I think the
> documentation
> for this setting needs an explanation of what the option does, as it is
> nearly
> impossible to find anything about this topic that would seem relevant when
> implementing an application which makes use of WebKit.
> 
> After some digging in the repository history I arrived at bug #112999 and
> bug #113122 and it took me wading through a pile of comments and reading
> some bits of the code to guess what the setting does. So please let's make
> the API documentation better by adding something in the lines of:
> 
>   “Enabling this setting will strip <script> tags from loaded HTML
>    content, but other forms of JavaScript execution e.g. using
>    webkit_web_view_run_javascript() are still allowed. This setting
>    is intended for applications which display HTML content but are
>    not full browsers, and which want to avoid the risk of script
>    injection attacks, as is the case of applications like e-mail
>    and news readers.”

The explanation in the patch is even more accurate and complete than this one, I would say:

+     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
+     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
+     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.

It's not only about script tags but also even listener attributes like onload and other js related attributes. That's clear in the current explanation. It also says that elements are removed while parsing and that js execution is still possible (not only run_js, but any js execution).

> If possible it should be more concrete that the above explaining what
> gets restricted and what not, because very often we programmers end up
> introducing accidental security vulnerabilities due to assumptions we
> make caused by incomplete documentation of third party code used.

I think it's clear enough. I'm open to change the name of the setting, though.
Comment 6 Adrian Perez 2019-01-15 06:25:24 PST
(In reply to Carlos Garcia Campos from comment #5)
> (In reply to Adrian Perez from comment #3)
> > Comment on attachment 359147 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=359147&action=review
> > 
> > Informally reviewing, this would be a r- for my: the description
> > in the API documentation is seriously lacking, and the name of the
> > setting is quite bad (non-descriptive, and difficult to search
> > online for). I don't care what WebKit calls the setting internally,
> > I would rather have a name for it that better indicates what it does
> > *and* that we provide a good description of what the setting does and
> > its intended usage.
> > 
> > > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> > > + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
> > 
> > What is “JavaScript Markup”? A specification, a WebKit specific concept?
> > Something widely understood among Web developers?... I think the
> > documentation
> > for this setting needs an explanation of what the option does, as it is
> > nearly
> > impossible to find anything about this topic that would seem relevant when
> > implementing an application which makes use of WebKit.
> > 
> > After some digging in the repository history I arrived at bug #112999 and
> > bug #113122 and it took me wading through a pile of comments and reading
> > some bits of the code to guess what the setting does. So please let's make
> > the API documentation better by adding something in the lines of:
> > 
> >   “Enabling this setting will strip <script> tags from loaded HTML
> >    content, but other forms of JavaScript execution e.g. using
> >    webkit_web_view_run_javascript() are still allowed. This setting
> >    is intended for applications which display HTML content but are
> >    not full browsers, and which want to avoid the risk of script
> >    injection attacks, as is the case of applications like e-mail
> >    and news readers.”
> 
> The explanation in the patch is even more accurate and complete than this
> one, I would say:
> 
> +     * Determines whether or not JavaScript markup is allowed in document.
> When this setting is disabled,
> +     * all JavaScript related elements and attributes are removed from the
> document during parsing. Note that
> +     * executing JavaScript is still allowed if
> #WebKitSettings:enable-javascript is %TRUE.

TBH, with this writing it is not completely clear to me what the
setting does. What does “all JavaScript related elements and attributes”
mean? For example it does not answer question like:

 - Can the JS DOM API be used to insert a new <script> tag?
 - If a <script> tag can be inserted using the DOM, will it work for
   <script src="..."> or only for elements with inline JS code in them? 
 - Will <script> tags inside nested frames or an <iframe> be loaded
   and executed?

(and that's only from the top of my head, I could come up with more)

> It's not only about script tags but also even listener attributes like
> onload and other js related attributes. That's clear in the current
> explanation. It also says that elements are removed while parsing and that
> js execution is still possible (not only run_js, but any js execution).

Which other means of execution other than *_run_javascript()? If “ALL
JavaScript related elements and attributes” can't be used (according to
the description, emphasis mine on “all”), how come that other means of
execution whih are NOT under control of the C API can result in JS
being executed?

> > If possible it should be more concrete that the above explaining what
> > gets restricted and what not, because very often we programmers end up
> > introducing accidental security vulnerabilities due to assumptions we
> > make caused by incomplete documentation of third party code used.
> 
> I think it's clear enough. I'm open to change the name of the setting,
> though.

To be honest, I am left with even more questions, so definitely not
“clear enough” for me. Not being clear and exhaustive explaining this
kind of things in the API documentation which affect security would
do a disservice, IMO.

My least of concerns is the name of the option, like I wrote already.

:-\
Comment 7 Carlos Garcia Campos 2019-01-15 06:35:25 PST
(In reply to Adrian Perez from comment #6)
> (In reply to Carlos Garcia Campos from comment #5)
> > (In reply to Adrian Perez from comment #3)
> > > Comment on attachment 359147 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=359147&action=review
> > > 
> > > Informally reviewing, this would be a r- for my: the description
> > > in the API documentation is seriously lacking, and the name of the
> > > setting is quite bad (non-descriptive, and difficult to search
> > > online for). I don't care what WebKit calls the setting internally,
> > > I would rather have a name for it that better indicates what it does
> > > *and* that we provide a good description of what the setting does and
> > > its intended usage.
> > > 
> > > > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> > > > + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
> > > 
> > > What is “JavaScript Markup”? A specification, a WebKit specific concept?
> > > Something widely understood among Web developers?... I think the
> > > documentation
> > > for this setting needs an explanation of what the option does, as it is
> > > nearly
> > > impossible to find anything about this topic that would seem relevant when
> > > implementing an application which makes use of WebKit.
> > > 
> > > After some digging in the repository history I arrived at bug #112999 and
> > > bug #113122 and it took me wading through a pile of comments and reading
> > > some bits of the code to guess what the setting does. So please let's make
> > > the API documentation better by adding something in the lines of:
> > > 
> > >   “Enabling this setting will strip <script> tags from loaded HTML
> > >    content, but other forms of JavaScript execution e.g. using
> > >    webkit_web_view_run_javascript() are still allowed. This setting
> > >    is intended for applications which display HTML content but are
> > >    not full browsers, and which want to avoid the risk of script
> > >    injection attacks, as is the case of applications like e-mail
> > >    and news readers.”
> > 
> > The explanation in the patch is even more accurate and complete than this
> > one, I would say:
> > 
> > +     * Determines whether or not JavaScript markup is allowed in document.
> > When this setting is disabled,
> > +     * all JavaScript related elements and attributes are removed from the
> > document during parsing. Note that
> > +     * executing JavaScript is still allowed if
> > #WebKitSettings:enable-javascript is %TRUE.
> 
> TBH, with this writing it is not completely clear to me what the
> setting does. What does “all JavaScript related elements and attributes”
> mean? For example it does not answer question like:
> 
>  - Can the JS DOM API be used to insert a new <script> tag?

Yes, that happens after parsing.

>  - If a <script> tag can be inserted using the DOM, will it work for
>    <script src="..."> or only for elements with inline JS code in them?

It should work either way.
 
>  - Will <script> tags inside nested frames or an <iframe> be loaded
>    and executed?

No.

> (and that's only from the top of my head, I could come up with more)
> 
> > It's not only about script tags but also even listener attributes like
> > onload and other js related attributes. That's clear in the current
> > explanation. It also says that elements are removed while parsing and that
> > js execution is still possible (not only run_js, but any js execution).
> 
> Which other means of execution other than *_run_javascript()?

Use scripts, js api in the web extension, user message handlers, etc.

> If “ALL
> JavaScript related elements and attributes” can't be used (according to
> the description, emphasis mine on “all”),

The description doesn't say they can't be used, it says they are removed from the document during parsing.

> how come that other means of
> execution whih are NOT under control of the C API can result in JS
> being executed?

Because other means of execution don't depend on script tags and attributes present in the document during parsing.

> > > If possible it should be more concrete that the above explaining what
> > > gets restricted and what not, because very often we programmers end up
> > > introducing accidental security vulnerabilities due to assumptions we
> > > make caused by incomplete documentation of third party code used.
> > 
> > I think it's clear enough. I'm open to change the name of the setting,
> > though.
> 
> To be honest, I am left with even more questions, so definitely not
> “clear enough” for me. Not being clear and exhaustive explaining this
> kind of things in the API documentation which affect security would
> do a disservice, IMO.

I'm fine with adding more explanations, but what you propose doesn't look better than the current text IMO.

> My least of concerns is the name of the option, like I wrote already.
> 
> :-\

I agree.
Comment 8 Michael Catanzaro 2019-01-20 20:33:59 PST
Comment on attachment 359147 [details]
Patch

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

Nice test.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1458
> +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,

documents.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1459
> +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that

JavaScript-related

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1460
> +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.

I agree with Adrian that this documentation in insufficient. Basically all of his questions should be clarified by better documentation.

But I have another concern. Executing JavaScript is still allowed... how? "Use scripts, js api in the web extension, user message handlers, etc." OK, but those are API requests, and I've separately suggests that API requests should always ignore #WebKitSettings:enable-javascript and execute anyway. This looked like something we could change on a cross-platform basis. So then maybe this new setting is not needed after all, if it, in practice, just blocks all JS except API requests? I really don't like mentioning that executing JavaScript is still allowed via API if #WebKitSettings:enable-javascript is %TRUE here, because I want to change that! Basically my fear is that "enable-javascript-markup" is the setting that most applications will want to use, and "enable-javascript" will be a wrong trap choice. So I'd rather make that usable first. Would this setting still be useful after that change?

r- to improve the docs and continue this discussion. The code is fine, of course.

>>>>> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
>>>>> + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
>>>> 
>>>> What is “JavaScript Markup”? A specification, a WebKit specific concept?
>>>> Something widely understood among Web developers?... I think the documentation
>>>> for this setting needs an explanation of what the option does, as it is nearly
>>>> impossible to find anything about this topic that would seem relevant when
>>>> implementing an application which makes use of WebKit.
>>>> 
>>>> After some digging in the repository history I arrived at bug #112999 and
>>>> bug #113122 and it took me wading through a pile of comments and reading
>>>> some bits of the code to guess what the setting does. So please let's make
>>>> the API documentation better by adding something in the lines of:
>>>> 
>>>>   “Enabling this setting will strip <script> tags from loaded HTML
>>>>    content, but other forms of JavaScript execution e.g. using
>>>>    webkit_web_view_run_javascript() are still allowed. This setting
>>>>    is intended for applications which display HTML content but are
>>>>    not full browsers, and which want to avoid the risk of script
>>>>    injection attacks, as is the case of applications like e-mail
>>>>    and news readers.”
>>>> 
>>>> If possible it should be more concrete that the above explaining what
>>>> gets restricted and what not, because very often we programmers end up
>>>> introducing accidental security vulnerabilities due to assumptions we
>>>> make caused by incomplete documentation of third party code used.
>>> 
>>> The explanation in the patch is even more accurate and complete than this one, I would say:
>>> 
>>> +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
>>> +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
>>> +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.
>>> 
>>> It's not only about script tags but also even listener attributes like onload and other js related attributes. That's clear in the current explanation. It also says that elements are removed while parsing and that js execution is still possible (not only run_js, but any js execution).
>> 
>> TBH, with this writing it is not completely clear to me what the
>> setting does. What does “all JavaScript related elements and attributes”
>> mean? For example it does not answer question like:
>> 
>>  - Can the JS DOM API be used to insert a new <script> tag?
>>  - If a <script> tag can be inserted using the DOM, will it work for
>>    <script src="..."> or only for elements with inline JS code in them? 
>>  - Will <script> tags inside nested frames or an <iframe> be loaded
>>    and executed?
>> 
>> (and that's only from the top of my head, I could come up with more)
> 
> Yes, that happens after parsing.

"%TRUE If" -> "%TRUE if"

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:437
> +        " <body onload='document.title = \"JavaScript allowed from body onload attribute\"'>"

JavaScript improperly allowed

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:439
> +        "  <script>document.title = 'JavaScript allowed from body script'</script>"

JavaScript improperly allowed

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:447
> +    g_assert(jsResult);

g_assert_nonnull
Comment 9 Carlos Garcia Campos 2019-01-21 00:00:35 PST
(In reply to Michael Catanzaro from comment #8)
> Comment on attachment 359147 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359147&action=review
> 
> Nice test.
> 
> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1458
> > +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
> 
> documents.

I mean the page document.

> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1459
> > +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> 
> JavaScript-related
> 
> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1460
> > +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> > +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.
> 
> I agree with Adrian that this documentation in insufficient. Basically all
> of his questions should be clarified by better documentation.

Ok, I'm open to suggestions.

> But I have another concern. Executing JavaScript is still allowed... how?

I think this is pretty clear too... JavaScript execution is not disallowed at all, the script elements and attributes are re moved from the document, that's the only thing.

> "Use scripts, js api in the web extension, user message handlers, etc." OK,
> but those are API requests, and I've separately suggests that API requests
> should always ignore #WebKitSettings:enable-javascript and execute anyway.

Yes, that's also desirable.

> This looked like something we could change on a cross-platform basis. So
> then maybe this new setting is not needed after all, if it, in practice,
> just blocks all JS except API requests?

That's a good question, I'm adding Geoffrey to the CC, since he suggested to use this setting instead of enable-js.

> I really don't like mentioning that
> executing JavaScript is still allowed via API if
> #WebKitSettings:enable-javascript is %TRUE here, because I want to change
> that! Basically my fear is that "enable-javascript-markup" is the setting
> that most applications will want to use, and "enable-javascript" will be a
> wrong trap choice. So I'd rather make that usable first. Would this setting
> still be useful after that change?

If removing js markup from document is a better approach we could just deprecate the other setting. I find more confusing a setting that disables js, but still allows it for API stuff, than a setting that removes js markup from document (but it seems this is only clear enough for me).

> r- to improve the docs and continue this discussion. The code is fine, of
> course.
> 
> >>>>> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> >>>>> + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
> >>>> 
> >>>> What is “JavaScript Markup”? A specification, a WebKit specific concept?
> >>>> Something widely understood among Web developers?... I think the documentation
> >>>> for this setting needs an explanation of what the option does, as it is nearly
> >>>> impossible to find anything about this topic that would seem relevant when
> >>>> implementing an application which makes use of WebKit.
> >>>> 
> >>>> After some digging in the repository history I arrived at bug #112999 and
> >>>> bug #113122 and it took me wading through a pile of comments and reading
> >>>> some bits of the code to guess what the setting does. So please let's make
> >>>> the API documentation better by adding something in the lines of:
> >>>> 
> >>>>   “Enabling this setting will strip <script> tags from loaded HTML
> >>>>    content, but other forms of JavaScript execution e.g. using
> >>>>    webkit_web_view_run_javascript() are still allowed. This setting
> >>>>    is intended for applications which display HTML content but are
> >>>>    not full browsers, and which want to avoid the risk of script
> >>>>    injection attacks, as is the case of applications like e-mail
> >>>>    and news readers.”
> >>>> 
> >>>> If possible it should be more concrete that the above explaining what
> >>>> gets restricted and what not, because very often we programmers end up
> >>>> introducing accidental security vulnerabilities due to assumptions we
> >>>> make caused by incomplete documentation of third party code used.
> >>> 
> >>> The explanation in the patch is even more accurate and complete than this one, I would say:
> >>> 
> >>> +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
> >>> +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> >>> +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.
> >>> 
> >>> It's not only about script tags but also even listener attributes like onload and other js related attributes. That's clear in the current explanation. It also says that elements are removed while parsing and that js execution is still possible (not only run_js, but any js execution).
> >> 
> >> TBH, with this writing it is not completely clear to me what the
> >> setting does. What does “all JavaScript related elements and attributes”
> >> mean? For example it does not answer question like:
> >> 
> >>  - Can the JS DOM API be used to insert a new <script> tag?
> >>  - If a <script> tag can be inserted using the DOM, will it work for
> >>    <script src="..."> or only for elements with inline JS code in them? 
> >>  - Will <script> tags inside nested frames or an <iframe> be loaded
> >>    and executed?
> >> 
> >> (and that's only from the top of my head, I could come up with more)
> > 
> > Yes, that happens after parsing.
> 
> "%TRUE If" -> "%TRUE if"
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:437
> > +        " <body onload='document.title = \"JavaScript allowed from body onload attribute\"'>"
> 
> JavaScript improperly allowed
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:439
> > +        "  <script>document.title = 'JavaScript allowed from body script'</script>"
> 
> JavaScript improperly allowed
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:447
> > +    g_assert(jsResult);
> 
> g_assert_nonnull
Comment 10 Michael Catanzaro 2019-01-21 12:46:12 PST
(In reply to Carlos Garcia Campos from comment #9)
> I think this is pretty clear too... JavaScript execution is not disallowed
> at all, the script elements and attributes are re moved from the document,
> that's the only thing.

Are there other ways to execute JavaScript, asides from script elements, script attributes, and API requests? I guess not?
 
> > This looked like something we could change on a cross-platform basis. So
> > then maybe this new setting is not needed after all, if it, in practice,
> > just blocks all JS except API requests?
> 
> That's a good question, I'm adding Geoffrey to the CC, since he suggested to
> use this setting instead of enable-js.

Basically I think we should fix bug #192753 one way or the other *first* and only *then* decide whether to add this setting. Because as I understand this setting, our solution to that bug might obviate the need for this setting.
Comment 11 Carlos Garcia Campos 2019-01-25 01:26:24 PST
(In reply to Michael Catanzaro from comment #10)
> (In reply to Carlos Garcia Campos from comment #9)
> > I think this is pretty clear too... JavaScript execution is not disallowed
> > at all, the script elements and attributes are re moved from the document,
> > that's the only thing.
> 
> Are there other ways to execute JavaScript, asides from script elements,
> script attributes, and API requests? I guess not?
>  
> > > This looked like something we could change on a cross-platform basis. So
> > > then maybe this new setting is not needed after all, if it, in practice,
> > > just blocks all JS except API requests?
> > 
> > That's a good question, I'm adding Geoffrey to the CC, since he suggested to
> > use this setting instead of enable-js.
> 
> Basically I think we should fix bug #192753 one way or the other *first* and
> only *then* decide whether to add this setting. Because as I understand this
> setting, our solution to that bug might obviate the need for this setting.

This is not so easy in the end. Maybe run_js should be allowed when js is disabled in settings, but FrameLoader doesn't dispatch didClearWindowObjectInWorld() when scripts are not allowed, so we can't inject our internal js api in the web process. There are more places where canExecuteScripts() is checked. I think it's by far a lot easier to explain and understand that script elements and attributes are removed from the document while parsing.
Comment 12 Michael Catanzaro 2019-01-25 07:41:42 PST
(In reply to Carlos Garcia Campos from comment #11)
> This is not so easy in the end. Maybe run_js should be allowed when js is
> disabled in settings, but FrameLoader doesn't dispatch
> didClearWindowObjectInWorld() when scripts are not allowed

So that means accessibility doesn't work when enable-js is disabled and nobody ever noticed this, right?
Comment 13 Michael Catanzaro 2019-01-25 07:43:08 PST
r=me with improved documentation, then. Maybe add a sentence or two listing some of the forms of JS execution that are blocked and not blocked by this setting.
Comment 14 Carlos Garcia Campos 2019-01-25 08:14:17 PST
(In reply to Michael Catanzaro from comment #12)
> (In reply to Carlos Garcia Campos from comment #11)
> > This is not so easy in the end. Maybe run_js should be allowed when js is
> > disabled in settings, but FrameLoader doesn't dispatch
> > didClearWindowObjectInWorld() when scripts are not allowed
> 
> So that means accessibility doesn't work when enable-js is disabled and
> nobody ever noticed this, right?

Accessibility? why?
Comment 15 Michael Catanzaro 2019-01-25 08:46:58 PST
(In reply to Carlos Garcia Campos from comment #14)
> Accessibility? why?

https://bugs.webkit.org/show_bug.cgi?id=182257#c18
Comment 16 Carlos Garcia Campos 2019-01-25 08:51:36 PST
(In reply to Michael Catanzaro from comment #15)
> (In reply to Carlos Garcia Campos from comment #14)
> > Accessibility? why?
> 
> https://bugs.webkit.org/show_bug.cgi?id=182257#c18

Oh, wow, a11y stuff doesn't belong to did clear window object indeed.
Comment 17 Carlos Garcia Campos 2019-02-10 22:35:42 PST
I'm going to land this before branching, we can improve the documentation in a follow up patch. Suggestions would be welcome, because current wording is still crystal clear for me.
Comment 18 Carlos Garcia Campos 2019-02-10 22:36:11 PST
Committed r241258: <https://trac.webkit.org/changeset/241258>
Comment 19 Michael Catanzaro 2019-02-11 07:07:35 PST
Comment on attachment 359147 [details]
Patch

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

>>> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1458
>>> +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
>> 
>> documents.
> 
> I mean the page document.

Still should be: "documents."