search

bost-ty / ff-lucid

A port of Daniel Eden's Lucid for Chrome to Firefox.
https://addons.mozilla.org/en-US/firefox/addon/ff-lucid/
1 stars 0 forks source link

Missing greetings text and notepad contents #6

Closed Hwiet closed 3 years ago

Hwiet commented 3 years ago

The greetings that show with the original Chrome extension (e.g. Good evening. It is Thursday, January 21) do not show with this add-on. The only thing I see on a new tab is the textarea with the placeholder "Write something", but nothing else.

I am using the latest Firefox version 84.0.2 (64-bit).

bost-ty commented 3 years ago

Hey! Thanks for bringing this up. This can also happen when Firefox Sync isn’t enabled — Lucid syncs your preferences (see the Options page in the extensions menu of Firefox) and relies on a time zone variable to generate the greeting.

I’ve had FF block my time zone detection due to the anti-fingerprinting flag and other built-in privacy features as well.

Issue #7 could be connected, we can investigate further.

On Jan 21, 2021, at 7:04 PM, Kyeu Sun Jo notifications@github.com wrote:

 The greetings that show with the original Chrome extension (e.g. Good evening. It is Thursday, January 21) do not show with this add-on. The only thing I see on a new tab is the textarea with the placeholder "Write something", but nothing else.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Hwiet commented 3 years ago

I've mentioned this in my other issue but I have my sync enabled.

I also tweaked the timezone offset on the options page but I believe it only fixes things if the time shown is wrong. In my case, the time doesn't show up at all.

bost-ty commented 3 years ago

Right on, thank you for getting back to me. If you check your Sync settings, is Add-on syncing enabled?

On Thu, Jan 21, 2021 at 7:46 PM Kyeu Sun Jo notifications@github.com wrote:

I've mentioned this in my other issue but I have my sync enabled.

I also tweaked the timezone offset on the options page but I believe it only fixes things if the time shown is wrong. In my case, the time doesn't show up at all.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bost-ty/firefox-lucid/issues/6#issuecomment-765101346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOT6DGRD4HLHK553TSY3Q5DS3DYKVANCNFSM4WN3AZVA .

Hwiet commented 3 years ago

Yes, it's enabled.

bost-ty commented 3 years ago

Right on. The greeting is populated by JavaScript, so that tells me something is going awry there. I'm about to install Firefox on a different system and install the add-on to see if I can replicate this!

Hwiet commented 3 years ago

Thank you for investigating this!

bost-ty commented 3 years ago

Looks like it matches #7 and #6 here using a Firefox with no account (and so no Sync of any kind).

The issue could lie in some assumptions around readStore and how that initializes the start function in the main override.js file.

However, I'm noticing that the Options page correctly saves and stores the color scheme & font preferences. I'm going to re-check how I implemented that. Super bizarre to see this working on my own machines but not on others, really intriguing.

bost-ty commented 3 years ago

An update: the data from the Options page is stored, but the main override.js for the page itself isn't collecting/populating/storing the data. Looks like the start function is the main culprit, and that the key being used isn't strictly necessary for sync storage with Firefox in general.

I need to block out time to rework this, which in the short term isn't super available.

@Hwiet , I'd love for you to be able to use this add-on; would you be interested in loading an older version of the plugin to see if the local storage capabilities work for you? It's pretty simple: just go to an older commit (maybe even the first one, just to be safe) and download the .ZIP of the repository. You can load that add-on going to "about:addons" in Firefox, clicking the gear, and using "Load addon from file". Pick the ZIP you downloaded.

That loaded addon will work until you restart Firefox -- a pain, I know -- but if it works we can implement the local storage functionality. Since my own system works with the addon, I think that having data in the addon could "kickstart" the sync and get it working for you. That's just a theory until I can get some time to really dig into this on the fresh install I have on another computer.

Hwiet commented 3 years ago

I loaded the addon from the very first commit as you said, and sure enough, the problems from this issue and #7 do not show up with this version. I can also see that the notepad text persists when the browser is restarted. Of course, I doubt this version would work with Firefox Sync.

(FYI Regular Firefox wouldn't let me install the downloaded addon, since it is unsigned, even when I set xpinstall.signatures.required to false in about:config; so I installed it on the Firefox Developer Edition. Also, Firefox kept saying the addon 'could not be installed because it appears to be corrupt'. This turned out to be a problem with how GitHub zips the files; I unzipped the download, which revealed a single folder. I was able to install the ZIP file when I zipped the contents of the folder instead of the folder itself.)

Would you be open to pull requests by any chance?

bost-ty commented 3 years ago

Amazing! I forgot they downloaded inside a folder, you were right on to zip the files that way. I'm glad that works.

I'm definitely open to pull requests, thank you for asking -- I would be very interested to see your code!

Re: the old version working with Sync, in the early versions there was local storage and sync (since it was converted from Chrome to FF Sync, mostly a permissions change and slight change of the browser.sync API for fetching and storing from sync). Having local storage with the key that Daniel initially setup (which I still use here for the notepad contents) should, I think, get the sync functionality started by allowing start to be called with at least some data in override.js.

In fact, that might be the place to start with getting functionality for new users: initializing some default data, even if it's blank, so that the check for d / data returns true. This might allow us to remove the key and a few more lines of code, since FF doesn't demand a unique key the way it's currently used in the code.

Again, really curious and excited to see your PR if you're interested. Cheers!

bost-ty commented 3 years ago

@Hwiet , an update: I'm thinking this fix will come as part of a full release to rework the way the notepad is saved and restored, unless you have any plans for a pull request (which is totally welcome). Work has taken all of my time lately, but I am still thinking about this and the best way to handle it to fix the issue once and for all. Thanks :)

bost-ty commented 3 years ago

Update: Started a new branch for this fix. So far, we're just touching the way the data is stored and retrieved and removing the key to simplify the chain.

bost-ty commented 3 years ago

Okay! I have a fix (again, we know that something working for me doesn't mean it works on every machine, for some reason).

If you can't update or don't get a fix through the about:addons interface, head to the product page itself and "Remove" and then "Add" firefox-lucid again.

Thanks for all of the help. I'll wait a day or two for an update from @Hwiet to see if this fixes the issue before closing.

bost-ty commented 3 years ago

Confirmed that this change works on a different system that was previously experiencing this bug. Yay! Fixes merged, closing issue. Please open a new issue if any bugs or issues occur in the new version. Update has been pushed to the Firefox Add-On Marketplace, and update is available via about:addons.