something odd about blocking Google Fonts #428

Closed
opened 5 months ago by lewisje · 1 comments
lewisje commented 5 months ago

I am the person who wrote the review on which this recent issue was based, and although the Chromium version of LocalCDN has not yet been updated to 2.6.6, I noticed that my workaround temporarily stopped working, and I needed to manually add any domain to the allow-list that I wanted Google Fonts to work on; I suspected that this line of code, which literally sets a certain configuration value to true even if the value it read from is false, was to blame:

    interceptor.blockGoogleFonts = items.blockGoogleFonts || true;

However, while I was trying to see whether this issue afflicted Firefox too, the problem went away, in both Firefox and Chrome, but that line of code above is still weird, unless possibly a false value for items.blockGoogleFonts would be read as the string "false" rather than the Boolean false.

I am the person who wrote the review on which [this recent issue](https://codeberg.org/nobody/LocalCDN/issues/416) was based, and although the Chromium version of LocalCDN has not yet been updated to 2.6.6, I noticed that my workaround temporarily stopped working, and I needed to manually add any domain to the allow-list that I wanted Google Fonts to work on; I suspected that [this line of code](https://codeberg.org/nobody/LocalCDN/src/branch/main/core/interceptor.js#L162), which literally sets a certain configuration value to `true` even if the value it read from is `false`, was to blame: ```javascript interceptor.blockGoogleFonts = items.blockGoogleFonts || true; ``` However, while I was trying to see whether this issue afflicted Firefox too, the problem went away, in both Firefox and Chrome, but that line of code above is still weird, unless possibly a false value for `items.blockGoogleFonts` would be read as the string `"false"` rather than the Boolean `false`.
Owner

Thank you for your report. This is a good example of why || is not always appropriate to set default values.

If the first value is false or undefined, the default value on the right is used.


This explains the problem in #343. Of course this cannot be reproduced with temporary profiles, because you change the value in the settings every time.

Thank you for your report. This is a good example of why `||` is not always appropriate to set default values. If the first value is `false` or `undefined`, the default value on the right is used. --- This explains the problem in #343. Of course this cannot be reproduced with temporary profiles, because you change the value in the settings every time.
nobody added the
bug
label 5 months ago
nobody added this to the v2.6.7 milestone 5 months ago
nobody closed this issue 5 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.