Opened 6 years ago

Closed 6 years ago

#1585 closed change (rejected)

Inconsistency between ReferrerMapping::BuildReferrerChain and comment for FilterEngine:: Matches

Reported by: sergz Assignee: sergz
Priority: Unknown Milestone:
Module: Libadblockplus Keywords:
Cc: fhd, trev Blocked By:
Blocking: #1564 Platform: Unknown
Ready: no Confidential: no
Tester: Verified working: no
Review URL(s):



For A->B->C->D (A loads B, B loads C, C loads D) we build referrer chain as [A, B, C] . On another hand, according to the comment it should be [C, B, A].
As well as tests show that it's [A, B, C]

What to change

Fix the impl of ReferrerMapping::BuildReferrerChain and fix the tests.

Change History (2)

comment:1 Changed 6 years ago by sergz

  • Status changed from new to reviewing

comment:2 Changed 6 years ago by fhd

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

It's just the comment that's wrong, everything else is fine. I've created #1586 for fixing it.

If we were to actually reverse the order in the code right now, we'd have to change it in two more places:

  1. FilterEngine::Matches would actually have to work the wrong way around.
  2. ReferrerMapping.cpp is currently duplicated in ABP for Android as

Anyway, IMO the current order (starting with the top-level frame) actually makes more sense when considering that this should represent the frame structure.

Note: See TracTickets for help on using tickets.