Skip to content

Conversation

@florian-schweiger
Copy link

This addresses #52.
Unfortunately, it does not solve the problem with the outlook.office.com
webapp and other sites that change their title after onComplete has fired.


Change the behaviour of the compareWithTitle option such that it is
treated the same as any of the ignore* options, namely as an additional
matching rule that contrains the results of the duplicate search even
further.

More precisely, now if option.compareWithTitle===true, then tabs with
the same URL (subject to the other matching rules) only match iff they
also share the same title.
Previously, matching in URLs alone would have sufficed.

Change the behaviour of the compareWithTitle option such that it is
treated the same as any of the ignore* options, namely as an additional
matching rule that contrains the results of the duplicate search even
further.

More precisely, now if option.compareWithTitle===true, then tabs with
the same URL (subject to the other matching rules) only match iff they
also share the same title.
Previously, matching in URLs alone would have sufficed.
@Peuj
Copy link
Owner

Peuj commented Feb 5, 2021

I understand your problem but it's a misunderstanding of the filters logic (my logic :) )
It's not "an additional matching rule that constrains the results", it's a more concatenation of the results.
The goal is not to limit but to get a larger scope.

If you select the filters Ignore hash part in URL and Ignore path part in URL, it will not limit the result to Ignore hash part in URL but return the result from Ignore path part in URL (which contains the result from Ignore hash part in URL ).

The same way, if you select only the filter Compare with title and the URLs don't match, it will not stop at the URLs comparison but it will also compare the titles.

So I will not change the behavior but I can add an option If you need a constraint rule.

@florian-schweiger
Copy link
Author

Ah ok, I see where you're coming from.

It's in part a language problem, isn't it?
Because a casual

"Hey, ignore A and ignore B"

translates to boolean algebra as

matches_ignoring_A OR matches_ignoring_B.

The colloquial "and" is in fact a boolean "OR"!

And that's how you treat the combination of rules, by concatenating them all with OR.

My misunderstanding came from the fact that while an "Ignore X" rule clearly widens the set of matching URLs (disabled = don't ignore X = fewer matches; enabled = ignore X = more matches), the "Compare with title" rule sounds as if it should shrink it (disabled = don't compare titles = all tabs match; enabled = compare titles = fewer matches).

May I suggest turning the "Compare with title" rule (off by default) into "Ignore title" (on by default)?
Then all rules would be of the same, widening kind.

@florian-schweiger
Copy link
Author

The same way, if you select only the filter Compare with title and the URLs don't match, it will not stop at the URLs comparison but it will also compare the titles.

It doesn't reach the title comparison though, or does it?

I just had another look at the code and the line that would accomplish this is worker.js:124.

But that line is never reached unless getTabs(queryInfo) on line 118 returns tabs that match queryInfo.url.

Am I missing something?

// worker.js

/*118*/ const openedTabs = await getTabs(queryInfo);
        if (openedTabs.length > 1) {
          const matchingObservedTabUrl = getMatchingURL(observedTabUrl);
          let match = false;
          for (const openedTab of openedTabs) {
            if ((openedTab.id === observedTab.id) || tabsInfo.isIgnoredTab(openedTab.id) || (isBlankURL(openedTab.url) && !isTabComplete(openedTab))) continue;
/*124*/     if ((getMatchingURL(openedTab.url) === matchingObservedTabUrl) || matchTitle(openedTab, observedTab)) { 
                match = true;
                //[...]
            }
          }
       //[...]
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants