Fix #8585 - Fix Javascript Disabling <NoScript>

Brandon-T commented 9 months ago

Summary of Changes

Fix Disabling JS not working due to a bug in iOS
Also, note that we do not need to refresh the page.
Changing it on the configuration directly works!

This pull request fixes #8585

Submitter Checklist:

[x] Unit Tests are updated to cover new or changed functionality
[x] User-facing strings use NSLocalizableString()
[x] New or updated UI has been tested across:
- [x] Light & dark mode
- [x] Different size classes (iPhone, landscape, iPad)
- [x] Different dynamic type sizes

Reviewer Checklist:

[ ] Issues include necessary QA labels:
- QA/(Yes|No)
- bug / enhancement
[ ] Necessary security reviews have taken place.
[ ] Adequate unit test coverage exists to prevent regressions.
[ ] Adequate test plan exists for QA to validate (if applicable).
[ ] Issue and pull request is assigned to a milestone (should happen at merge time).

diracdeltas commented 9 months ago

IIRC this has broken before too. Can we add an automated test for it?

Brandon-T commented 9 months ago

IIRC this has broken before too. Can we add an automated test for it?

We can, but the problem is that this is OS related since the API gets updated when the OS gets updated (WebKit ships with the OS), so the unit test would only tell us when the OS has changed the behaviour.

Is that acceptable?

iccub commented 9 months ago

pinging @diracdeltas about the comment Brandon wrote, easy to miss notifications during the winter break

diracdeltas commented 8 months ago

thx for the reminder. that sgtm. also i should have clarified adding a test does not block this PR.

jparedeslopez commented 8 months ago

@anthonypkeane was this fix released to prod already? We need it to fix the issue with images.

iccub commented 8 months ago

@jparedeslopez next week most likely

brave / brave-ios