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

Follow up to #22203: Fix custom Brave notification ads should be attached to the browser window #30508

Open
btlechowski opened this issue May 23, 2023 · 5 comments
Assignees
Labels

Comments

@btlechowski
Copy link

btlechowski commented May 23, 2023

Follow up to #22203. Thanks to @stephendonner for catching this on macOS

Steps to Reproduce

  1. Clean install
  2. Run Brave with: --enable-logging=stderr --vmodule="*/variations/*"=6,"*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=9,"*/brave_user_model/*"=6,"*/bat_ads/*"=6 --brave-ads-staging --rewards=staging=true --variations-server-url=https://v --enable-features="CustomAdNotifications:should_attached_ad_notification_to_browser_window/true"
  3. Enable rewards and ads
  4. Trigger a notification ad
  5. Make another application active

Note: use --enable-features="CustomNotificationAds for 1.53.x

Actual result:

Notification ad is not anchored to the browser window, the ad stays on top

22203 linux

Expected result:

Notification ad is anchored to the browser window

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.53.62 Chromium: 114.0.5735.26 (Official Build) nightly (64-bit)
Revision 7075cbb66f0542ac3e01ddfde6b813e7d61118a5-refs/branch-heads/5735@{#454}
OS Ubuntu 18.04 LTS

cc @tmancey @aseren

@btlechowski btlechowski added priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/exclude feature/ads OS/Desktop labels May 23, 2023
@btlechowski
Copy link
Author

Reproduced on Windows

Brave 1.52.109 Chromium: 114.0.5735.26 (Official Build) beta (64-bit)
Revision 7075cbb66f0542ac3e01ddfde6b813e7d61118a5-refs/branch-heads/5735@{#454}
OS Windows 10 Version 22H2 (Build 19045.2965)

22203 windows

@stephendonner
Copy link

Also experienced this on macOS using

Brave 1.52.108 Chromium: 114.0.5735.26 (Official Build) beta (x86_64)
Revision 7075cbb66f0542ac3e01ddfde6b813e7d61118a5-refs/branch-heads/5735@{#454}
OS macOS Version 11.7.7 (Build 20G1345)

Launched using --enable-logging=stderr --vmodule="*/variations/*"=6,"*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=9,"*/brave_user_model/*"=6,"*/bat_ads/*"=6 --brave-ads-staging --rewards=staging=true --variations-server-url=https://v --enable-features="CustomAdNotifications:should_attached_ad_notification_to_browser_window/true"

no-minimize

@LaurenWags
Copy link
Member

fwiw, I have a dual monitor set up. the custom ad notification is shown on my main monitor screen while I have Brave open on my secondary monitor. Would that be changed when this is resolved?

@aseren
Copy link

aseren commented May 23, 2023

There were lots of changes that happened with feature naming.

For Beta 1.52x the attached notification parameter name was changed to should_attach_ad_notification_to_browser_window here:
brave/brave-core#18076

For Nightly parameter was renamed to use_same_z_order_as_browser_window and set to true by default. So there is no need to set it through the command line.

The attached custom notifications do not work for Linux for now. This should be fixed here: #29744

cc @btlechowski @stephendonner

@aseren
Copy link

aseren commented May 23, 2023

fwiw, I have a dual monitor set up. the custom ad notification is shown on my main monitor screen while I have Brave open on my secondary monitor. Would that be changed when this is resolved?

@LaurenWags, yes. The attached custom notification should fix this issue.

@tmancey tmancey added this to Ads Jun 10, 2024
@tmancey tmancey moved this to Done in Ads Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

4 participants