Closed balmas closed 5 years ago
please test at http://www-test.alpheios.net/alpheios-texts/Perseus.text.1999.02.0066/book1_poem1.html When you load the page, Alpheios should be active, but the panel should not open.
1) when I first open the link, I confirm that tools work and no info panel is generated 2) on page reload, the red icon is on and info panel is generated.
I cannot reproduce that behavior Using http://www-test.alpheios.net/alpheios-texts/Perseus.text.1999.02.0066/book1_poem1.html and the Safari build.2.1.0.16 (Safari version 12.0). I have tried the following:
A:
B:
Also tested in Chrome and FF, same behavior.
(Note that in Safari, if after 6 you return to the original page from step 1, Alpheios does not reactivate itself as it does in FF and Chrome, but I can live with that.)
@monzug can you clear your browser cache and try again just to be sure?
@balmas try the following: on the embedded library test site, double click on any word, open the diagram and the inflection table then click on the alpheios icon (you should see a glance of red icon or a flickering), then click on reload. the red icon is on and the info page is generated. if u cannot repro, then I will clean and reinstall the safari build tomorrow.
confirm that on reload I got the info panel and the red icon on. this is from build 2.1.0.16 installed through the installer and with test site http://www-test.alpheios.net/alpheios-texts/Perseus.text.1999.02.0066/book1_poem1.html
also, there is Alpheios Activate in the context menu which turn on and immediately off the tools
I can reproduce it on http://www-test.alpheios.net/alpheios-texts/Perseus.text.1999.02.0066/book1_poem1.html, this is not, however, happening for me in my tests on my dev server on pages with embedded library on.
The panel that is opened in that test is the one of the Safari app extension. We can tell that by the version number within an info panel. This is probably happening because Safari app extension cannot detect a presence of the embedded library, not because embedded library opens a panel incorrectly.
Since I can reproduce it, will work on the fix (if this will be possible within a Safari app extension architecture).
@balmas: I've made some tests, and I think I understand the root cause. This is timing, mostly.
To put it in a long way, right now there are two methods to detect an embedded lib: with a body
attribute and with a DOM message. We have several levels of embedded lib detection now:
The problem in Safari architecture is that the content script is injected into the page automatically, and we do not know exactly when this happens. Each page usually has one or several JS script sourced in. Their loading starts after HTML of the page is parsed (to not go into too many details). But loading those scripts could take different time for each script and for all of them, depending on where are they located and how large their sizes are. If there are too many scripts and CSS files on the page, their loading is delayed even more due to the limit of max number of resource request sent simultaneously (usually eight at a time in modern browsers), so the scripts queue up.
If seems that Safari app extension injects a content script right after the page is parsed and loading of the scripts sourced by the page is started (but not necessarily finished yet). This is a source of non-determinism that creates so many problems for us. If the scripts on the page are small and loaded from fast local resources (as in my local tests), they (including an embedded lib) will be loaded before the content script. In this situation, content script will be able to recognize an embedded lib, because it's already there. If, however, page scripts are loaded slowly, content script will not discover an embedded library because the library is not loaded yet.
This is what probably happening in this test performed by Monica. Because it is a remote page with a slow resource loading, content script is loaded before an embedded library is there. So the sequence of operations is: O1. Check (1) fails: embedded lib is not loaded yet. O2. Check (2) fails: we send a message, but because embedded lib is not loaded it goes to nowhere. O3. Content script sends a "contentReady" message to background indicating it is ready. O4. As it is a reload, background script sends a state request to content script asking it to restore its previous state. O5. State request executes its state request handler. It has an additional safety check for the embedded lib attribute. But because (O4) is happening fast (right after reload), embedded lib is still might be not there. So that check fails too. And, if background wants state to be "active", content script does so. O6. Embedded lib is loaded finally, but the content script does not know about it yet, until the next state request, when a content script will check for an embedded lib attribute during check (3). This will happen only during incoming state request from the background. So if to click on the Alpheios button several times, it will deactivate itself finally. But that makes a terrible user experience.
The problem here is that some sites can be really slow in loading all its resources. I've seen sites that took 30 or more seconds to do so (!). This means that if an embedded lib loading will be delayed for that long, we will be able to deactivate webextension with a very huge delay only.
I'm not sure what's the best way to deal with all this. Probably (or, actually, for sure) there is no ideal solution (but how much do I wish we would find one!). One measure we could use is probably make an embedded lib fire an event every time it is activated. As a content script listens to that, it can disable itself reactively. This might result in extension being activated and then, after some delay, become disabled on its own. As puzzling as it might be to the user, it's still probably better than the current behavior. To ease user confusion, we can put a noticeable narrow bar at the top of the page (as sites do withe messages about cookies these days) saying something about extension being disabled because an embedded library instance was loaded by the page.
Really need to think about it more. What do you think about it?
@kirlat and @monzug I think we need to tackle this with the next big release but not for the Safari release. As of right now, the number of sites using the embedded library is very small, and we are not actively promoting our own pages. This will change going forward and we do need to address it one way or another, but I do not want to hold the 2.1.0.x release for it to work perfectly.
One measure we could use is probably make an embedded lib fire an event every time it is activated.As a content script listens to that, it can disable itself reactively. This might result in extension being activated and then, after some delay, become disabled on its own. As puzzling as it might be to the user, it's still probably better than the current behavior. To ease user confusion, we can put a noticeable narrow bar at the top of the page (as sites do withe messages about cookies these days) saying something about extension being disabled because an embedded library instance was loaded by the page.
If this is not more than a few hours work, I would be okay with this solution for right now
Please retest in Build 2.1.0.17 (in Dropbox and https://github.com/alpheios-project/webextension/tree/qa-2.1.0.17/dist) using http://www-test.alpheios.net/alpheios-texts/Perseus.text.1999.02.0066/book1_poem1.html for the embedded library test page. Clear your browser cache first to be sure to get the updated code on the embedded page.
I haven't been able to reproduce this myself, but you should see a message if the page with the embedded library loads after the extension is already activated.
message is generated on reload of the embedded library test page and the red icon turns on and immediately off. However, I got the info panel instead of the message (see attachment), so there is still something a little off. @balmas, I think we can live with this for now. what do you think?
yes willing to live with that.
per my comments in alpheios-project/webextension#178, the embedded library needs to be able to instruct the UIController that the panel should not be open upon activation, effectively overriding the panelOnActivate setting, which should not be impacted one way or another by this.