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

Fixed tab grouping broken under Vivaldi 5.3 #11

Merged
merged 1 commit into from Jul 31, 2023

Conversation

grigorye
Copy link
Contributor

Starting with at least Vivaldi 5.3.2679.51, grouping functionality no longer works correctly. From what I could figure out while debugging, the code no longer runs Vivaldi-specific paths, as extData is no longer part of the tab objects.

Still, there's vivExtData, that looks like a new name for the same field:

Screen Shot 2022-06-10 at 23 15 38

This PR adjusts the code for the new field name, and from what I can see, as result, the grouping functionality is working fine again.

@grigorye grigorye changed the title Updated for VivaldiTab.extData renamed to VivaldiTab.vivExtData. Fixed tab grouping broken under Vivaldi 5.3 Jun 10, 2022
Copy link
Contributor

@djdv djdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been using this patch for over a week now and it seems to work fine.
In addition to the grouping, this appears to resolve #7 as well.

The change is a straightforward reference update.
My only gripe is that I can't seem to find documentation for Vivaldi's extension API, which I assume is not publicized.
Given that this change works, it's reasonable to assume this is the correct reference name (at least at this time).

I have no influence on this repo but can give a Works on my machine™ seal of approval if that's useful to the maintainer. And big thanks to both authors (patch and original extension).

@jdodsoncollins
Copy link

I'd also like to confirm that this works reliably and as expected within Vivaldi. Thank you for submitting this.

mochibuta added a commit to mochibuta/chrome-otto-tabs that referenced this pull request Nov 8, 2022
@djdv djdv mentioned this pull request Jan 14, 2023
@borsini
Copy link
Owner

borsini commented Jul 31, 2023

Thanks for this fix 🙏

@borsini borsini merged commit 3bb5ba9 into borsini:master Jul 31, 2023
borsini added a commit that referenced this pull request Aug 6, 2023
Loading this extension as-is throws a warning about v2's deprecation.
This is in preparation for:
https://developer.chrome.com/docs/extensions/mv3/mv2-sunset/
Following:
https://developer.chrome.com/docs/extensions/mv3/mv3-migration/#man-sw

The extension's version number should probably be incremented as well.
(Either in this commit or one directly following it.)

I'm running this on Vivalid and it seems to work (when on top of
#11), but otherwise this
is not extensively tested.

***

Extra note:
It doesn't seem required, but it might be better to move
`background.js`'s content to another file.
Then change `bacground.js` to something that looks like the (now
deleted) `background.html` page, except using `importScripts` instead of
`<script>` tags.
As-is, `background.ts` seems to import `./tabs_helpers.js`
anyway/explicitly so it doesn't seem to make a difference when iusing
`"type": "module"`.

Co-authored-by: Benjamin Orsini <github.o7tgl@simplelogin.com>
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.

None yet

5 participants