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

ads tabClassification is empty after browser restart #2875

Closed
btlechowski opened this issue Jan 9, 2019 · 2 comments
Closed

ads tabClassification is empty after browser restart #2875

btlechowski opened this issue Jan 9, 2019 · 2 comments
Assignees
Labels
feature/ads priority/P1 A very extremely bad problem. We might push a hotfix for it. QA/Yes

Comments

@btlechowski
Copy link

Steps to Reproduce

  1. Enable rewards
  2. Enable ads in brave://rewards/
  3. Open new tab and load https://www.iab.com/
  4. Restart the browser

Actual result:

tabClassification is empty
"tabClassification":[]
log:

[15148:10184:0109/212427.479:INFO:ads_service_impl.cc(958)] AdsService Event Log: {"data":{"type":"load","stamp":"2019-01-09T21:24:27Z","tabId":11,"tabType":"click","tabUrl":"https://www.iab.com/","tabClassification":[]}

I believe this will prevent some ads from showing.

Note: reload or two of the tab fixes the issue

Expected result:

tabClassification is populated
"tabClassification":["business","advertising"]
log:

[15148:10184:0109/212427.479:INFO:ads_service_impl.cc(958)] AdsService Event Log: {"data":{"type":"load","stamp":"2019-01-[15148:10184:0109/212816.743:INFO:ads_service_impl.cc(958)] AdsService Event Log: {"data":{"type":"load","stamp":"2019-01-09T21:28:16Z","tabId":11,"tabType":"click","tabUrl":"https://www.iab.com/","tabClassification":["business","advertising"]}

Reproduces how often:

always

Brave version (brave://version info)

Brave 0.60.9 Chromium: 72.0.3626.28 (Official Build) dev (64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Windows

Checked on Windows 7 and Windows 10

cc: @bridiver @jsecretan @brave/legacy_qa

@btlechowski btlechowski added this to the 1.x Backlog milestone Jan 9, 2019
@mrose17 mrose17 added the priority/P1 A very extremely bad problem. We might push a hotfix for it. label Jan 15, 2019
@tmancey tmancey self-assigned this Jan 17, 2019
@mrose17
Copy link
Member

mrose17 commented Jan 17, 2019

after talking with @tmancey about this, the entry is that the log entry is "wrong". what's going on here is this: when the browser restarts, it re-opens previously open tabs (if configured to do so). those tabs are not run through the categorization code, because we don't want to double-count them. however, the log entry simply spits out what it knows "now", not what it knew "then".

because this diagnostic is misleading, we will fix the diagnostic to reflect reality.

@mrose17 mrose17 assigned mrose17 and unassigned tmancey Jan 17, 2019
@mrose17
Copy link
Member

mrose17 commented Jan 21, 2019

Fixed by brave-intl/bat-native-ads#106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ads priority/P1 A very extremely bad problem. We might push a hotfix for it. QA/Yes
Projects
None yet
Development

No branches or pull requests

3 participants