Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Desktop] Move cosmetic filtering script injection logic from extension to native #10582

Closed
simonhong opened this issue Jul 7, 2020 · 4 comments

Comments

@simonhong
Copy link
Member

simonhong commented Jul 7, 2020

With script injection in native, we can share it with android also.

Now, script injection is done at brave extension's content script.
I think we coud mov script injection to native by observing webcontent's loading state.

chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
  const action = typeof msg === 'string' ? msg : msg.type
  switch (action) {
    case 'cosmeticFilteringBackgroundReady': {
      if (msg.hideOptions !== undefined) {
        scheduleQueuePump(msg.hideOptions.hide1pContent, msg.hideOptions.generichide)
      }
      injectScriptlet(msg.scriptlet)
      break
    }

   ...
}
@antonok-edm
Copy link
Collaborator

antonok-edm commented Jul 7, 2020

It may be helpful to reference brave/brave-core#5860, which adds a single hardcoded scriptlet injection on youtube.com for Android devices. That can be safely removed once general-purpose injection is working correctly.

@simonhong simonhong added this to the 1.13.x - Nightly milestone Jul 10, 2020
@simonhong
Copy link
Member Author

Upstream only allows using content::RenderFrameHost::AllowInjectingJavaScript(); in only restricted configuration.
According to below comment, we should not use this for destkop and android also.
But, I'm not sure why upstream doesn't allow...

  // Globally allows for injecting JavaScript into the main world. This feature
  // is present only to support Android WebView, WebLayer, Fuchsia web.Contexts,
  // and CastOS content shell. It must not be used in other configurations.
  static void AllowInjectingJavaScript();

@simonhong
Copy link
Member Author

@bridiver Could we call content::RenderFrameHost::AllowInjectingJavaScript() in desktop or android? Or we should avoid it?

@simonhong simonhong removed this from the 1.13.x - Nightly milestone Jul 13, 2020
@antonok-edm antonok-edm added the closed/duplicate Issue has already been reported label Jul 28, 2021
@antonok-edm
Copy link
Collaborator

This was completed a while ago, desktop is currently running the native cosmetic filtering backend by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants