Opened 8 days ago

Last modified 8 days ago

#6838 reviewing defect

Sometimes all subscriptions are listed as additional subscriptions in Firefox

Reported by: Ross Assignee: greiner
Priority: P1 Milestone:
Module: User-Interface Keywords:
Cc: sebastian, kzar, greiner, agiammarchi, saroyanm, wspee Blocked By:
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Unknown Verified working: no
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 8 days ago.

Download all attachments as: .zip

Change History (6)

Changed 8 days ago by Ross

comment:1 Changed 8 days ago by Ross

  • Description modified (diff)

comment:2 Changed 8 days 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 8 days 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 8 days ago by greiner

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

comment:5 Changed 8 days 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

Note: See TracTickets for help on using tickets.