search

hypothesis / h

Annotate with anyone, anywhere.
https://hypothes.is/
BSD 2-Clause "Simplified" License
2.94k stars 427 forks source link

Selection / highlighting breaks across <ul>'s #2010

Closed BigBlueHat closed 9 years ago

BigBlueHat commented 9 years ago

This is probably a poor description and someone more familiar with Annotator and it's DOM manipulation / handling could say it better (and feel free to!).

Here's the situation: On http://fivethirtyeight.com/features/how-the-fivethirtyeight-senate-forecast-model-works/ The bulleted list is...crazy. It's actually three separate <ul>'s each with a single <li> (for whitespace? :confused: )

From what I can tell, the selection adding logic doesn't make it back that far in the hierarchy to create the selection <span>'s in the DOM...or something.

I used Firefox's HTML inspector and made it one single <ul> and all worked as expected.

Not sure what would need to be changed about Annotator to make it bridge that gap.

Also, I'm happy to move this issue to the Annotator repo if that's better.

csillag commented 9 years ago

What exactly do I need to select to reproduce this?

And what is the actual symptom that you see?

tilgovi commented 9 years ago

Is anyone else okay with me beginning to close all tickets related to problems induced by the highlighter markup and pushing for nickstenning/marks work instead? @nickstenning ?

csillag commented 9 years ago

Is anyone else okay with me beginning to close all tickets related to problems induced by the highlighter markup and pushing for nickstenning/marks work instead? @nickstenning ?

  1. What's the " nickstenning/marks work " ?
  2. I think we should not close anything until we have fixed the problem, one way or other.
tilgovi commented 9 years ago
  1. Strangely, github didn't linkify it: https://github.com/nickstenning/marks
  2. Yes we should, if what we decide is to implement a different thing which will fix it. We don't need several things that report specific problems that all have a root cause when the root cause is being addressed by a different issue.
BigBlueHat commented 9 years ago

@tilgovi select text across a couple bullet points--which will span the ul's. The annotation won't "stick."

As far as closing these go, we need to have a stated / determined replacement (marks...maybe?) and a plan for realizing it before close these. When we close them, it should be either because they're fixed or are now being tested with the new solution or being reported as feature requirements for the new thing. We shouldn't just loose them as they're at least useful test cases.

tilgovi commented 9 years ago

Then we should garden/consolidate. On Mar 16, 2015 10:42 PM, "BigBlueHat" notifications@github.com wrote:

@tilgovi https://github.com/tilgovi select text across a couple bullet points--which will span the ul's. The annotation won't "stick."

As far as closing these go, we need to have a stated / determined replacement (marks...maybe?) and a plan for realizing it before close these. When we close them, it should be either because they're fixed or are now being tested with the new solution or being reported as feature requirements for the new thing. We shouldn't just loose them as they're at least useful test cases.

— Reply to this email directly or view it on GitHub https://github.com/hypothesis/h/issues/2010#issuecomment-82057652.

tilgovi commented 9 years ago

I can't reproduce any problem on that page with that markup. If you can, please provide greater detail on the expected and actual outcomes and what you did.