-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Bug 1797577 - Add cookie banner handling panel to the toolbar. #28044
Conversation
9fff964
to
e17b085
Compare
Request for data collection review formAll questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection.
|
e17b085
to
0f4a81c
Compare
Data Review
Yes, through the metrics.yaml file and the Glean Dictionary
Yes, through the "Send Usage Data" preference in the application settings
N/A, collection set to end or be renewed by version 118
Category 2, Interaction data
default-on
No
Yes
No Resultdata-review+ |
* | ||
* @param context [Context] used for various Android interactions. | ||
* @param sessionId [String] The current session ID. | ||
* @param websiteUrl [String] the URL of the current web page. | ||
* @param isTrackingProtectionEnabled [Boolean] Current status of tracking protection | ||
* for this session. | ||
* @param isCookieHandlingEnabled [Boolean] Current status of cookie banner handling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @param isCookieHandlingEnabled [Boolean] Current status of cookie banner handling. | |
* @param isCookieHandlingEnabled [Boolean] Current status of cookie banner handling |
I have started looking at this and I think I can also pick up the review since I am familiar with the code here since I worked on it with Arturo on some of the changes back in MR 2021. |
just passing by, top padding inside the second pair of screenshots looks cropped. but I can't find the design for the reference |
You can find the mockups here https://www.figma.com/file/FAZxkK9IRLlCqyhXBqId7e/Auto-Cookie-Banner-Consent-Desktop-%26-Mobile?node-id=910%3A27683&t=biamx5dzxBpgvX9r-0 |
Hi @Amejia481 , I tested the feature and I found three bugs . |
Screen_Recording_20221206-145407_Firefox.Fenix.mp4 |
app/src/main/res/layout/fragment_cookie_banner_handling_details_dialog.xml
Show resolved
Hide resolved
Screen_Recording_20221206-172434_Firefox.Fenix.mp4 |
app/src/main/java/org/mozilla/fenix/settings/quicksettings/protections/ProtectionsView.kt
Show resolved
Hide resolved
Thanks for taking a look :) !
Yeah, it looks like there a bug when we reload 🐞 , the same happens when you click the three dotted menu and press the reload icon, please feel free to file a bug.
This matches the same behaviour that Firefox desktop nightly has. You could verify by downloading desktop nightly and activating the cookie banner handling UI ( |
app/src/main/java/org/mozilla/fenix/trackingprotection/ProtectionsStore.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/mozilla/fenix/trackingprotection/ProtectionsStore.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionPanelView.kt
Outdated
Show resolved
Hide resolved
...enix/settings/quicksettings/protections/cookiebanners/CookieBannerHandlingDetailsViewTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a very brief review and nothing sticks out to me that hasn't already been mentioned. I will defer the final review to @iorgamgabriel
0f4a81c
to
9992f16
Compare
And for the incognito tab, it doesn't work. |
Screen_Recording_20221208-131658_Firefox.Fenix.mp4 |
9992f16
to
1056a0e
Compare
In this case, this is the expected behaviour same as Firefox desktop :) See #28044 (comment) for more details |
1056a0e
to
f9eb635
Compare
So if for private mode doesn't work what is the point of this feature in Focus ? Only when you stay in screen , it's kind of useless. @Amejia481 |
Yeah, at the moment it's only for collecting feedback until the platform ticket is addressed |
Cookie banner handling is ON
Cookie banner handling is OFF
Cookie banner handling is ON
Cookie banner handling is OFF
Pull Request checklist
QA
To download an APK when reviewing a PR (after all CI tasks finished running):
Checks
at the top of the PR page.firefoxci-taskcluster
group on the left to expand all tasks.build-debug
task.View task in Taskcluster
in the newDETAILS
section.GitHub Automation
Used by GitHub Actions.