Opened 4 months ago

Closed 4 months ago

Last modified 7 weeks ago

#6838 closed defect (fixed)

Sometimes all subscriptions are listed as additional subscriptions

Reported by: Ross Assignee: greiner
Priority: P1 Milestone:
Module: User-Interface Keywords:
Cc: sebastian, kzar, greiner, agiammarchi, saroyanm, wspee Blocked By:
Blocking: #6892 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockplusui/merge_requests/83

Description (last modified by Ross)

Environment

ABP 3.2.2102
Firefox 61 / 55 / 51 / Windows 10

Could not reproduce in Chrome 61 / 49.

Does not occur every time.

How to reproduce

  1. Install ABP.
  2. Open options.
  3. Close options.
  4. Navigate to http://testpages.adblockplus.org
  5. Select the subscribe link.
  6. Add the subscription.
  7. Close options.
  8. Open options.

Observed behaviour

Sometimes after adding what ABP considers "an additional subscription" the ABP options page seems to get confused and sometimes displays all of the installed subscriptions as additional subscriptions.

This means that UI elements (such as language, Anti-CV etc) are also displaying the wrong state.

See attached screenshot.

Sometimes the following error is displayed in the console:

Type error for parameter tabId (Integer -1 is too small (must be at least 0)) for tabs.get. background.js:146

Expected behaviour

Only actual additional subscriptions to be listed in the additional subscriptions section.

Attachments (1)

6838-Subscriptions.jpg (256.0 KB) - added by Ross 4 months ago.

Download all attachments as: .zip

Change History (11)

Changed 4 months ago by Ross

comment:1 Changed 4 months ago by Ross

  • Description modified (diff)

comment:2 Changed 4 months ago by greiner

  • Cc agiammarchi saroyanm wspee added
  • Ready set

On first glance, this sounds like a race condition since we may not have finished loading subscriptions.xml yet at the point when we're being passed the filter list to add.

I suspect the error message to be unrelated though.

Note that I don't see any indication of this being a regression to the previous release in case this makes a difference for the ongoing release.

comment:3 Changed 4 months ago by greiner

  • Owner set to greiner

I was able to reproduce this behavior in Chrome by delaying the loadRecommendations() function by a second, confirming my suspicion of it being a race condition.

comment:4 Changed 4 months ago by greiner

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:5 Changed 4 months ago by greiner

FYI: Dave and I agreed on IRC to not include this fix in the currently ongoing release since the issue doesn't appear to be a regression.

<greiner> kzar: the question is just whether you want to include the fix in the ongoing release since it's not technically a regression… my plan would be to get it through review and make it available as a dependency update shortly after in case we want to include it

<kzar> greiner: So this behaviour was also present in 3.2?

<greiner> kzar: I'd have to trace it back but it appears to have been present since the initial release of the new options page… I can double-check that if you want

<kzar> greiner: Yes please, if it's not a pain. If you can reproduce for 3.2 then it's no longer a release blocker

<greiner> kzar: unfortunately, I can't reproduce the issue based on the given instructions so all I have to go off of is the race condition I spotted that produces the given results by delaying the loading of subscriptions.xml… for that I can verify that (a) it happens on both Firefox and Chrome and (b) that it also happens with version 3.2

<kzar> greiner: OK, well if you can reproduce that race condition which produces the described symtoms on 3.2 as well I think that's good enough. I don't consider it a release blocker, what do you think?

<greiner> kzar: I'd agree, especially since we barely contributed any UI code to 3.3 due to the rush

comment:6 Changed 4 months ago by greiner

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:7 Changed 4 months ago by greiner

  • Blocking 6892 added

comment:9 Changed 3 months ago by greiner

  • Platform changed from Firefox to Unknown / Cross platform
  • Summary changed from Sometimes all subscriptions are listed as additional subscriptions in Firefox to Sometimes all subscriptions are listed as additional subscriptions

Marked ticket as being cross-platform.

comment:10 Changed 7 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Could not reproduce misplaced filter lists at all.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

Note: See TracTickets for help on using tickets.