Skip to content

Commit

Permalink
Merge pull request #1846 from macrogreg/bookmarks-bar-separators
Browse files Browse the repository at this point in the history 
Separators on Bookmarks-bar are Vertical rather than Horizontal
  • Loading branch information
@marcelklehr
marcelklehr authored Feb 1, 2025
2 parents 9e3eacd + 88389ef commit 6683981
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 37 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## [5.4.4]

### Fixed
* fix(SyncProcess): When creating dummy bookmarks representing separators, make sure to use vertical lines on the Toolbar, and horizontal lines otherwise.

## [5.4.3]

### Fixed
Expand Down
Empty file modified manifest-firefox-override.sh
100755 → 100644
View file
Empty file.
3 changes: 3 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
"description": "Sync your bookmarks privately across browsers and devices",
"scripts": {
"build": "NODE_OPTIONS=--max-old-space-size=5000 gulp",
"build-win": "SET NODE_OPTIONS=--max-old-space-size=5000 & gulp",
"build-release": "NODE_OPTIONS=--max-old-space-size=5000 gulp release",
"build-release-win": "SET NODE_OPTIONS=--max-old-space-size=5000 & gulp release",
"watch": "NODE_OPTIONS=--max-old-space-size=5000 gulp watch",
"watch-win": "SET NODE_OPTIONS=--max-old-space-size=5000 & gulp watch",
"test": "node --unhandled-rejections=strict test/selenium-runner.js",
"lint": "eslint --ext .js,.vue src",
"lint:fix": "eslint --ext .js,.vue src --fix"
Expand Down
29 changes: 20 additions & 9 deletions src/lib/browser/BrowserTree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ export default class BrowserTree implements IResource<typeof ItemLocation.LOCAL>
private absoluteRoot: { id: string }
private absoluteRootPromise: Promise<void>

static readonly TITLE_BOOKMARKS_BAR: string = 'Bookmarks Bar'
static readonly TITLE_OTHER_BOOKMARKS: string = 'Other Bookmarks'
static readonly TITLE_BOOKMARKS_MENU: string = 'Bookmarks Menu'
static readonly TITLE_MOBILE_BOOKMARKS: string = 'Mobile Bookmarks'
static readonly TITLE_SEPARATOR_HORZ: string = '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯'
static readonly TITLE_SEPARATOR_VERT: string = ''

constructor(storage:unknown, rootId:string) {
this.rootId = rootId
this.storage = storage
Expand All @@ -40,7 +47,8 @@ export default class BrowserTree implements IResource<typeof ItemLocation.LOCAL>
await this.absoluteRootPromise
const allAccounts = await (await Account.getAccountClass()).getAllAccounts()

const recurse = (node, parentId?, rng?) => {
const recurse = (node, parentId?, isOnToolbar?, rng?) => {

if (
allAccounts.some(
acc => acc.getData().localRoot === node.id && String(node.id) !== String(this.rootId) && !acc.getData().nestedSync
Expand All @@ -49,22 +57,23 @@ export default class BrowserTree implements IResource<typeof ItemLocation.LOCAL>
// This is the root folder of a different account and the user doesn't want nested sync
return
}
let overrideTitle, isRoot
let overrideTitle, isRoot, isToolbar
if (node.parentId === this.absoluteRoot.id && !isVivaldiBrowser) {
switch (node.id) {
case '1': // Chrome
case 'toolbar_____': // Firefox
overrideTitle = 'Bookmarks Bar'
overrideTitle = BrowserTree.TITLE_BOOKMARKS_BAR
isToolbar = true
break
case '2': // Chrome
case 'unfiled_____': // Firefox
overrideTitle = 'Other Bookmarks'
overrideTitle = BrowserTree.TITLE_OTHER_BOOKMARKS
break
case 'menu________': // Firefox
overrideTitle = 'Bookmarks Menu'
overrideTitle = BrowserTree.TITLE_BOOKMARKS_MENU
break
case 'mobile______': // Firefox
overrideTitle = 'Mobile Bookmarks'
overrideTitle = BrowserTree.TITLE_MOBILE_BOOKMARKS
}
if (overrideTitle) {
Logger.log(
Expand All @@ -91,7 +100,7 @@ export default class BrowserTree implements IResource<typeof ItemLocation.LOCAL>
title: parentId ? overrideTitle || node.title : undefined,
children: node.children
.map((child) => {
return recurse(child, node.id, rng)
return recurse(child, node.id, isToolbar, rng)
})
.filter(child => !!child) // filter out `undefined` from nested accounts
})
Expand All @@ -103,10 +112,12 @@ export default class BrowserTree implements IResource<typeof ItemLocation.LOCAL>
location: ItemLocation.LOCAL,
id: node.id,
parentId,
title: '-----',
title: isOnToolbar ? BrowserTree.TITLE_SEPARATOR_VERT : BrowserTree.TITLE_SEPARATOR_HORZ,
// If you have more than a quarter million separators in one folder, call me
// Floccus breaks down much earlier atm
url: `https://separator.floccus.org/?id=${rng.int(0,1000000)}`,
url: 'https://separator.floccus.org/' +
(isOnToolbar ? 'vertical.html' : '') +
`?id=${rng.int(0,1000000)}`,
})
} else {
return new Tree.Bookmark({
Expand Down
144 changes: 122 additions & 22 deletions src/test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ import * as AsyncParallel from 'async-parallel'
import DefunctCrypto from '../lib/DefunctCrypto'
import Controller from '../lib/Controller'
import FakeAdapter from '../lib/adapters/Fake'
import BrowserTree from '../lib/browser/BrowserTree'

chai.use(chaiAsPromised)
const expect = chai.expect

let expectTreeEqual = function(tree1, tree2, ignoreEmptyFolders, checkOrder = true) {
expectTreeEqualRec(tree1, tree2, 0, ignoreEmptyFolders, checkOrder)
}

let expectTreeEqualRec = function(tree1, tree2, recDepth, ignoreEmptyFolders, checkOrder) {
try {
expect(tree1.title).to.equal(tree2.title)
if (tree2.url) {
Expand All @@ -40,12 +45,12 @@ let expectTreeEqual = function(tree1, tree2, ignoreEmptyFolders, checkOrder = tr
: tree2.children
expect(children1).to.have.length(children2.length)
children2.forEach((child2, i) => {
expectTreeEqual(children1[i], child2, ignoreEmptyFolders, checkOrder)
expectTreeEqualRec(children1[i], child2, recDepth + 1, ignoreEmptyFolders, checkOrder)
})
}
} catch (e) {
console.log(
`Trees are not equal: (checkOrder: ${checkOrder}, ignoreEmptyFolders: ${ignoreEmptyFolders})`,
`Trees are not equal: (recDepth: ${recDepth}, checkOrder: ${checkOrder}, ignoreEmptyFolders: ${ignoreEmptyFolders})\n`,
'Tree 1:\n' + tree1.inspect(0) + '\n',
'Tree 2:\n' + tree2.inspect(0)
)
Expand Down Expand Up @@ -2297,6 +2302,102 @@ describe('Floccus', function() {
false
)
})
it('should convert vertical and horizontal separators', async function() {
if (BROWSER !== 'firefox') {
this.skip()
return
}

// Remove all nodes except the system nodes:
const deleteNonSysNodes = async(delNodeId) => {
let delChildren = await browser.bookmarks.getChildren(delNodeId)
for (const delChild of delChildren) {
await deleteNonSysNodes(delChild.id)
}
if (!delNodeId.endsWith('_____')) {
await browser.bookmarks.remove(delNodeId)
}
}
await deleteNonSysNodes('root________')

await browser.bookmarks.create({
title: 'url1',
url: 'http://url1/',
parentId: 'menu________'
})
await browser.bookmarks.create({
type: 'separator',
parentId: 'menu________'
})
const toolbarNameNormalFolder = await browser.bookmarks.create({
title: BrowserTree.TITLE_BOOKMARKS_BAR,
parentId: 'menu________'
})
await browser.bookmarks.create({
title: 'url2',
url: 'http://url2/',
parentId: toolbarNameNormalFolder.id
})
await browser.bookmarks.create({
type: 'separator',
parentId: toolbarNameNormalFolder.id
})

await browser.bookmarks.create({
title: 'url3',
url: 'http://url3/',
parentId: 'toolbar_____'
})
await browser.bookmarks.create({
type: 'separator',
parentId: 'toolbar_____'
})
const onToolbarNormalFolder = await browser.bookmarks.create({
title: 'A Folder',
parentId: 'toolbar_____'
})
await browser.bookmarks.create({
title: 'url4',
url: 'http://url4/',
parentId: onToolbarNormalFolder.id
})
await browser.bookmarks.create({
type: 'separator',
parentId: onToolbarNormalFolder.id
})

let brTree = new BrowserTree('Dummy Storage', 'root________')
let bmTree = await brTree.getBookmarksTree()

expectTreeEqual(
bmTree,
new Folder({title: undefined,
children: [
new Folder({title: 'Bookmarks Menu',
children: [
new Bookmark({title: 'url1', url: 'http://url1/'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=242649'}),
new Folder({title: 'Bookmarks Bar',
children: [
new Bookmark({title: 'url2', url: 'http://url2/'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=591710'}),
]}),
]}),
new Folder({title: 'Bookmarks Bar',
children: [
new Bookmark({title: 'url3', url: 'http://url3/'}),
new Bookmark({title: '', url: 'https://separator.floccus.org/vertical.html?id=616887'}),
new Folder({title: 'A Folder',
children: [
new Bookmark({title: 'url4', url: 'http://url4/'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=890296'}),
]}),
]})
]}),
true
)
await deleteNonSysNodes('root________')
})
it('should sync separators', async function() {
if (ACCOUNT_DATA.noCache) {
this.skip()
Expand All @@ -2310,7 +2411,6 @@ describe('Floccus', function() {
return this.skip()
}
const localRoot = account.getData().localRoot

const barFolder = await browser.bookmarks.create({
title: 'bar',
parentId: localRoot
Expand Down Expand Up @@ -2352,11 +2452,11 @@ describe('Floccus', function() {
new Folder({title: 'foo',
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=731368'})
]}),
]}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=731368'})
]})
]})
]}),
false
)
Expand All @@ -2369,11 +2469,11 @@ describe('Floccus', function() {
new Folder({title: 'foo',
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=731368'})
]}),
]}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=731368'})
]})
]})
]}),
false
)
Expand Down Expand Up @@ -2401,10 +2501,10 @@ describe('Floccus', function() {
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'})
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'})
]}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=379999'})
]}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=379999'})
]})
]}),
false
)
Expand All @@ -2419,10 +2519,10 @@ describe('Floccus', function() {
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=731368'})
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=731368'})
]}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'})
]}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'})
]})
]}),
false
)
Expand Down Expand Up @@ -2482,9 +2582,9 @@ describe('Floccus', function() {
new Folder({title: 'foo',
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=731368'})
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=731368'})
]}),
]}),
]}),
Expand All @@ -2499,9 +2599,9 @@ describe('Floccus', function() {
new Folder({title: 'foo',
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=731368'})
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=731368'})
]}),
]}),
]}),
Expand All @@ -2528,7 +2628,7 @@ describe('Floccus', function() {
children: [
new Bookmark({title: 'url', url: 'http://ur.l/'}),
new Bookmark({title: 'url2',url: 'http://ur2.l'}),
new Bookmark({title: '-----', url: 'https://separator.floccus.org/?id=467366'})
new Bookmark({title: '⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯', url: 'https://separator.floccus.org/?id=467366'})
]}),
]}),

Expand Down
8 changes: 4 additions & 4 deletions src/ui/components/AccountCard.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@
</template>
<template v-if="status === 'scheduled'">
<v-btn
:color="statusType"
class="float-right"
x-small
@click="onForceSync">
:color="statusType"
class="float-right"
x-small
@click="onForceSync">
{{ t('LabelScheduledforcesync') }}
</v-btn>
</template>
Expand Down
4 changes: 2 additions & 2 deletions src/ui/components/OptionsGoogleDrive.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@
</v-card-title>
<v-card-text>
<OptionAutoSync
:value="enabled"
@input="$emit('update:enabled', $event)" />
:value="enabled"
@input="$emit('update:enabled', $event)" />
<OptionSyncInterval
:value="syncInterval"
@input="$emit('update:syncInterval', $event)" />
Expand Down

0 comments on commit 6683981

Please sign in to comment.