Use data uris to store the images for the profiles #113
Conversation
…me and firefox on my machine
…) and use the data uris instead of the original img urls
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
==========================================
- Coverage 89.06% 88.25% -0.82%
==========================================
Files 9 9
Lines 311 315 +4
==========================================
+ Hits 277 278 +1
- Misses 34 37 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
==========================================
- Coverage 89.06% 88.88% -0.18%
==========================================
Files 9 9
Lines 311 315 +4
==========================================
+ Hits 277 280 +3
- Misses 34 35 +1
Continue to review full report at Codecov.
|
|
I think the bad practice to store an image as data-uri on the configuration because it is limited by storage spec very much. I recognize the care of an image property about security too. However, I think that we should not use the image property if concerned about it. |
|
Is there a particular number of entries this extension is required or is hoped to support? The localStorage space limit in Chrome is 5MB unless you request the "unlimitedStorage" permission. On Firefox storage is unlimited pending the implementation of limits and the "unlimitedStorage" permission. I have 16 entries in my configuration, each with an image and it uses ~66KB of local storage. That is ~4KB per entry including the image data. Assuming my images are indicative of real world usage and rounding 4KB up to 5KB an entry I can store 1024 roles before running out of storage. Is storing >1024 roles a realistic use case for this extension? Please keep in mind that since images are optional, users with > 1024 roles need not associate images with each role (would anybody bother to do this?). Other users may be happy to trade the number of roles that can be stored against the ability to have images load faster and not leak to AWS on every page load. Or we could add the "unlimitedStorage" permission and not worry about it? Reworking this to use the file storage API (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Working_with_files) seems substantially more effort and more change for little upside. |
AESR supports the max 200 entries.
AESR currently uses the sync storage, not the local storage. I think that images should be shared in your team with AWS S3, and I am thinking "Should the entries be displayed from the browser menu button, rather than expanding the list in AWS MC?". |
|
Ah, okay, I see that now in options.js. Limited to 100KB. I notice also that the 'rawdata' is retained in localStorage (thus my confusion). How does the sync storage relate to the local storage? And how should it? It appears as though the local storage is simply overwritten by each sync get if the data retrieved from the sync is not null/undefined. Would an acceptable solution be to not sync the image data (but include the image urls) and require the configuration to be re-saved (and the images re-fetched into data uris saved in local storage) for each browser the user has?
I personally like the way the extension integrates and feels part of the AWS console. I use quite a few extensions and often the AESR icon is hidden in the overflow menu. Particularly when on a laptop screen. |
|
With the update to have the ability to choose local storage now would this be a good option to help people manage different accounts? |
tilfin
force-pushed
the
main
branch
2 times, most recently
from
b58bfb3
to
9a7ceff
Compare
Store the images in the configuration as data-uris. The data uris can either be entered directly by users or will be generated when the configuration is saved based on the image attribute. This resolves many CSP issues where each img src would result in a security exception being sent to AWS. It is also faster as the only network requests happen when the configuration is saved and not when the menu is expanded or the page loaded. The images are stored as 64x64 pngs so I am not sure of the effect this has on the number of entries that can be stored. This presumes the fix for bug #111 has already been merged to master as it is included.