Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multiple unrelated Hypothesis frames on one page causes havoc #3433

Closed
nickstenning opened this issue Jun 9, 2016 · 2 comments
Closed

Multiple unrelated Hypothesis frames on one page causes havoc #3433

nickstenning opened this issue Jun 9, 2016 · 2 comments

Comments

@nickstenning
Copy link
Contributor

Steps to reproduce

  1. Create an HTML document with multiple frames, each containing an instance of Hypothesis. For example, create multiple iframes to different https://via.hypothes.is pages. The frames must all be hosted on the same origin.

Expected behaviour

Each independent Hypothesis instance can be used on its own without interfering with the others.

Actual behaviour

The "magic" autodiscovery features cause complete havoc. Specifically:
@nickstenning nickstenning changed the title Multiple unrelated Hypothesis frames on one page causes havoc. Multiple unrelated Hypothesis frames on one page causes havoc Jun 9, 2016
@robertknight
Copy link
Member

I think this might be the same issue as seen in #2829

@robertknight robertknight mentioned this issue Jun 9, 2016
Closed
robertknight added a commit to hypothesis/client that referenced this issue Sep 23, 2016
@robertknight
Remove the AnnotationUISync service
0b570b2 
Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
@robertknight robertknight mentioned this issue Sep 23, 2016
Merged
robertknight added a commit to hypothesis/client that referenced this issue Oct 12, 2016
@robertknight
Remove the AnnotationUISync service
ebb878f 
Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
robertknight added a commit to hypothesis/client that referenced this issue Oct 25, 2016
@robertknight
Remove the AnnotationUISync service
e7e3059 
Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
robertknight added a commit to hypothesis/client that referenced this issue Nov 7, 2016
@robertknight
Remove the AnnotationUISync service
54789ba 
Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
robertknight added a commit to hypothesis/client that referenced this issue Nov 8, 2016
@robertknight
Remove the AnnotationUISync service
42d9aba 
Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
sheetaluk pushed a commit to hypothesis/client that referenced this issue Nov 8, 2016
@robertknight @sheetaluk
Remove AnnotationUISync service (#127)
bb71cd7 
* Move event handlers out of AnnotationUISync

As a step towards having all bridge event handlers in one place, move
these event handlers into frameSync.

 * Add tests for findIDsForTags() and for re-exporting of this
   function from annotationUI

* Move sidebarOpened event handler to FrameSync service

* Remove the AnnotationUISync service

Its functionality has now been moved into the FrameSync service, except
for the logic that was intended to synchronize the "Show Annotation
Highlights" logic across multiple frames, since this is clearly broken
[1].

The two pieces of functionality related to this we need to support at
the moment are:

 1. The `showHighlights` config option to set whether highlights are
    initially visible.

 2. The highlight toggle button in the sidebar's outer frame

Both of these work without the sidebar app's involvement.

[1] See hypothesis/h#3433 and
    hypothesis/h#3295
@nickstenning nickstenning mentioned this issue Feb 21, 2017
Closed
@nickstenning
Copy link
Contributor Author

Moved to hypothesis/client#249.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@robertknight @nickstenning