Add cross-origin annotatable iframe dev server scenario #3732
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3732 +/- ##
=======================================
Coverage 98.95% 98.95%
=======================================
Files 211 211
Lines 7671 7671
Branches 1731 1731
=======================================
Hits 7591 7591
Misses 80 80 Continue to review full report at Codecov.
|
| @@ -316,6 +316,8 @@ gulp.task('serve-package', () => { | |||
|
|
|||
| gulp.task('serve-test-pages', () => { | |||
| serveDev(3000, { clientUrl: `//{current_host}:3001/hypothesis` }); | |||
| // Starts an additional dev web server to test cross-origin functionality | |||
| serveDev(3002, { clientUrl: `//{current_host}:3001/hypothesis` }); | |||
There was a problem hiding this comment.
I think this is useful in other context, like for example serving PDFs files from another URL and checking that the fingerprint association allows still to find the annotations.
| let port = new URL(document.location).port === '3000' ? '3002' : '3000'; | ||
| let url = '//localhost:' + port + '/document/burns-embedded'; | ||
| document.querySelector('iframe').setAttribute('src', url); |
There was a problem hiding this comment.
It is not the most terse JS because JS is not transpiled and I wanted to make it compatible with all the browsers we support.
Related to #2900
|
Starting an additional local server on a different port seems reasonable. Could also be useful for testing anchoring based on non-URL identifiers (eg. DOIs) (Edit: I see you already wrote a comment to this effect). I suggest using a smaller document for this test unless you really need a long document. Very long files run into issues with things like GitHub's diff viewer not displaying the diff by default. |
esanzgar
force-pushed
the
cross-origin-guest-frame
branch
from
533b0e4 to
939ade8
Compare
|
I changed the document for |
There was a problem hiding this comment.
LGTM. I think it would be useful to adjust the messages that get logged when the dev server starts up, to make it more obvious what the difference is between the different servers. Currently the messages read:
[10:08:01] Package served at http://localhost:3001/hypothesis
[10:08:01] Dev web server started at http://localhost:3000/
[10:08:01] Dev web server started at http://localhost:3002/
I think it would be better if these messages read something like:
[10:08:01] Client boot script served at http://localhost:3001/hypothesis
[10:08:01] Primary dev server started at http://localhost:3000/
[10:08:01] Alternate dev server started at http://localhost:3002/
| <iframe></iframe> | ||
| <script> | ||
| // Set a different port for the iframe `src` property to make it cross-origin. | ||
| let port = new URL(document.location).port === '3000' ? '3002' : '3000'; |
There was a problem hiding this comment.
You can use document.location.port here, since there is a Location.port property.
The current inter-frame communication doesn't work if an annotatable (guest) iframe is from a different origin than the host frame (see #3611 (comment)). This will be fixed in a more comprehensive overhaul of the inter-face communication (see #3533). Meanwhile, I add a scenario into the local dev server where the annotatable iframe is from an origin different than the host frame. For this, I needed to spawn an additional dev server at port 3002): ``` [11:32:50] Dev web server started at http://localhost:3000/ [11:32:50] Dev web server started at http://localhost:3002/ ``` Close #3629
Edge 17 complains if the script is outside the body.
esanzgar
force-pushed
the
cross-origin-guest-frame
branch
from
ccd20e5 to
9c6c4df
Compare
The current inter-frame communication doesn't work if an annotatable
(guest) iframe is from a different origin than the host frame (see
#3611 (comment)). This
will be fixed in a more comprehensive overhaul of the inter-face
communication (see #3533).
Meanwhile, I add a scenario into the local dev server where the
annotatable iframe is from a different origin than the host frame. For
this, I needed to spawn an additional dev server at port 3002):
Close #3629