Skip to content

Conversation

@Phoenix-
Copy link
Contributor

@Phoenix- Phoenix- commented Feb 29, 2024

Fix for #2598

Problem Description

Bookmarks from Opera browser are not being indexed unless placed onto bookmarks toolbar.
Checked for Opera 107.0.5045.36 but I dont think its linked to any particular version.
Bookmarks file for opera differs from Chrome one, note 'custom_root' sub-object

{
   "roots": {
      "bookmark_bar": {
         "children": [  ]
      },
      "custom_root": {
         "pinboard": {
            "children": [  ]
         },
         "speedDial": {
            "children": [ 
               {
                  "children": [  ]
               } 
            ]
         },
         "trash": {
            "children": [  ]
         },
         "unsorted": {
            "children": [  ]
         },
         "unsyncedPinboard": {
            "children": [  ]
         },
         "userRoot": {
            "children": [ {
               "children": [ {
                  "name": "...",
                  "type": "url",
                  "url": "https://..."
               } ],
               "name": "...",
               "type": "folder"
            },
<...>

Current implementation of EnumerateFolderBookmark method skips any object without 'children' field.
My fix slightly changes EnumerateRoot method to check for possible 'custom_root' field and if found treat it as a root itself.
Should not break other browsers and fixes Opera.

This fix takes "custom_root" sub-object into account.

@taooceros
Copy link
Member

interesting, will any other browser being affected? I feel like it is weird to modify the chromiumloader

@Phoenix-
Copy link
Contributor Author

Phoenix- commented Mar 1, 2024

Nothing else should be affected. The behavior changes only when tag with name 'custom_root' is encountered,

@Phoenix-
Copy link
Contributor Author

Phoenix- commented Mar 1, 2024

Another way exists to fix the issue. I can write another implementation of IBookmarkLoader for Opera. But it will differ from Chromium one by literally 2 lines of code. Not sure it is a good idea.

@taooceros
Copy link
Member

I think it's ok for now. Could you add a comment for that? If we have more exception then we consider more abstraction.

@Phoenix-
Copy link
Contributor Author

Phoenix- commented Mar 1, 2024

Added comment with details.

@github-actions

This comment has been minimized.

Fix opera bookmarks - comment

Fix opera bookmarks - comment
@VictoriousRaptor VictoriousRaptor added this to the 1.17.3 milestone Mar 2, 2024
@VictoriousRaptor VictoriousRaptor added the bug Something isn't working label Mar 2, 2024
@VictoriousRaptor
Copy link
Contributor

LGTM. BTW could you please copy the problem description part of #2598 to this PR?

@VictoriousRaptor VictoriousRaptor merged commit 7d171a3 into Flow-Launcher:dev Mar 2, 2024
@Phoenix-
Copy link
Contributor Author

Phoenix- commented Mar 2, 2024

Added description

@jjw24 jjw24 modified the milestones: 1.17.3, 1.18.0 Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants