[navigation #2] AdClick attribution navigation Tab Extension #886
Conversation
|
@mallexxx, the code looks good to me 👍 Unfortunately, I am not very familiar with AdClick attribution. Could you be more specific in testing steps? Or we can ask someone from O-E to test. |
|
@mallexxx these changes look good, but overall Attribution does not seem to work well across tabs, e.g. inherited attribution is not passed when I open new tab from a tab where attribution is active, e.g. when you are on a vendor site and open particular item details in a new tab. That does not seem related to this PR, but rather to some of the other changes/refactoring. |
mallexxx
changed the title
mallexxx
changed the title
mallexxx
changed the title
…r behaviour; cleanup
There was a problem hiding this comment.
Congrats, really nice suite of tests :) I've tested this and it feels like everything is in order.
I think we could use one more test scenarios (that I think are not being checked):
- On tracker data being updated, make sure that attribution rules are actually applied correctly if attribution was present. There was a bug like that in the past.
- On navigation to different vendor, attribution rules should be updated with new ones.
|
@bwaresiak as we discussed online, I have no motivation atm for adding the test you mentioned, let‘s consider it done for now if it works and schedule a follow-up project for adding the Integration Tests that‘ll run through the whole privacy test pages setup |
* canGoBack, canGoForward RePublished, Tab.publishers refactored * redo redirect(_:NavigationAction, invalidatingBackItemIfNeeded) * tests * don‘t use shared state in tests * fix RELEASE * rollback renaming * drop RePublished * fix header name * [navigation #4] SERP headers navigation responder (#887) * SERP headers handling in SerpHeadersNavigationResponder
Task/Issue URL: https://app.asana.com/0/0/1203487090719126/f
Tech Design URL: https://app.asana.com/0/481882893211075/1203268245242140/f
CC: @tomasstrba @bwaresiak
Description:
Navigation handling for AdClick attribution moved to the AdClick Tab Extension
Steps to test this PR: