Skip to content

Conversation

@this-is-shivamsingh
Copy link

@this-is-shivamsingh this-is-shivamsingh commented Jul 26, 2025

Issue: #91

Context:

  • Showing correct error message where there is invalid json file extension or invalid json content

Changes:

  • Added a validation check in github-bookmark-loader.js to check for following things
    • file extension
    • data content length
    • json content datatype
      to throw appropriate errors with correct filename
  • Show correct filename for which the error is thrown
  • in options.js show check the error.message data type before showing it on pop window to avoid [object object] error message

Testing:

  • Tested give fix and some other flows on Firefox, Brave and Chrome

…ow appropriate errors

fix: check for err.message type before displaying to user
Comment on lines +137 to +140
return {
name: decodeURIComponent(fileName),
data: JSON.parse(file.data),
};
Copy link
Author

@this-is-shivamsingh this-is-shivamsingh Jul 26, 2025

Choose a reason for hiding this comment

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

  • In bookmarksync.js earlier we were only sending file data, and not the file name, therefore while showing error message it used to like this
The bookmarks file with name <name not defined> is not valid
  • This will be get fixed now for firefox, chrome and brave browser as well

// Github only gives url of files under a folder, so we need to extract file name from url
// example github url: https://api.github.com/repos/this-is-shivamsingh/bookmarks/contents/test_folder_3%2Fa.json
// Then full file name is after `contents` part
const fileName = url.split('contents').pop();
Copy link
Author

@this-is-shivamsingh this-is-shivamsingh Jul 26, 2025

Choose a reason for hiding this comment

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

  • Tested with multiple nested folders, and it successfully extracts file name

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.

1 participant