search

haskell / haddock

Haskell Documentation Tool
www.haskell.org/haddock/
BSD 2-Clause "Simplified" License
361 stars 242 forks source link

Odd in-page search behavior with recent Chromium and Chrome #1451

Closed treeowl closed 2 years ago

treeowl commented 2 years ago

I don't know if this is a Chromium bug or a Haddock bug or some weird interaction, but since I updated Chrome and Chromium recently, I've been having a problem. Specifically, if I hit Ctrl-F to find a word in some module documentation, and the word appears some ways down the page, it ends up opening up the Synopsis tab and searching there before continuing with the main page. UGH. Note: I don't have this problem in Firefox.

eltix commented 2 years ago

I confirm I've been getting the same visualization bug in latest Chrome. Here is a screenshot exhibiting the bug:

image

Clicking on the search match in the yellow right panel has no effect

stevehartdata commented 2 years ago

This is a result of a recent change to the HTML spec that specifies that closed details elements should be searchable. See the relevant section in the HTML spec, which was changed in this pull request. The change was implemented in Chrome 97 and Firefox has an open bug to implement it.

treeowl commented 2 years ago

Thanks, @stevehartdata. That confirms the bug is in Haddock and not Chromium. Do you happen to know what we have to do to fix it? The search feature is currently nearly unusable.

stevehartdata commented 2 years ago

I think it depends on what the desired functionality is. It's not obvious to me that the new functionality is always undesirable, though I can see why it would be annoying for certain use cases.

If the goal is just to revert back to the old behavior so that the synopsis is not searchable when hidden, then we can toggle a CSS property like visibility or content-visibility to hidden whenever the synopsis is collapsed. (I would need to do some more research to see which property is most appropriate.) But I'm not sure that this is the right approach, since we'd be working around the default semantics for the details element. Perhaps we would want to also change the element type to something other than details.

I'm not a regular contributor to Haddock, so I don't know who would make the call on what the desired behavior is. Are you the one?

treeowl commented 2 years ago

No, I'm certainly not. Have you tried navigating Hackage with the new behavior? I can confidently say it's not what anyone is likely to want.

treeowl commented 2 years ago

Specifically: search will go into the sidebar, then clicking on the found thing won't actually take you there. It's just wretched.

stevehartdata commented 2 years ago

The links in the synopsis not taking you to the expected location is a different bug that is not related to the recent HTML change. I definitely agree that that behavior is not how it should be, and I've opened #1457 to track that issue separately.

treeowl commented 2 years ago

I personally really don't want searching to open the sidebar either.

MaxGabriel commented 2 years ago

For pages with wide type signatures, this issue makes searching almost unusable. In Eltix's screenshot of text, it's sort like, "oh this side bar popped up, that's annoying and weird", but for eg persistent it's more like "my entire screen was taken over by this popping up and I can't search"

image
parsonsmatt commented 2 years ago

I can't really say how I feel about this without being hyperbolic and rude, so I'll just say "please let's fix it"

Lysxia commented 2 years ago

This does seem like an easy fix. I'll make a PR.

The new default behavior of <details> makes sense as a default, but this certainly seems like a case where we want to override it. There's no reason to search the synopsis element, since all of its contents can already be found elsewhere in the main part of the page. It doesn't seem possible to unconditionally hide an element from search, so disabling visibility when it's already hidden by <details> seems the next best option. And it seems visibility: hidden is the right setting; content-visibility: hidden is too new, not supported on Firefox, even though it seems preferable in principle because we do not care about preserving its layout.

MaxGabriel commented 2 years ago

@Lysxia Thank you so much!

eltix commented 2 years ago

@Kleidukos @Lysxia Thank you very much for the fix. What remains to do in order to have the fix effective in hackage?

Kleidukos commented 2 years ago

@eltix Hackage will have do adopt the next Haddock release, which will be cut for the next GHC release. :)

eltix commented 2 years ago

@Kleidukos I don't see any issue tracking this on hackage's side. Opening a random hackage page shows that they're using Haddock version 2.13.2 while the latest version seems to be 2.25.1. How is this synchronized in practice? I can take care of opening an issue on hackage's side to tell them to upgrade to latest haddock but maybe I am just misunderstanding how this works.

Kleidukos commented 2 years ago

How is this synchronized in practice?

This is done at the discretion of the Hackage maintainers, you'll have to ask them directly

Lysxia commented 2 years ago

That's a very old package. Keep in mind that the footer records the haddock that generated it, so it does not reflect the current version of haddock on Hackage. Picking a more recent package like text shows that 2.24 is being used. I believe hackage-server is where the issue can be raised. Individual packages may also upload their own docs built with their own haddock.

eltix commented 2 years ago

Thank you both for your explanations. If I understood correctly, we can only raise the issue hackage-server once haddock has made a new release.

mrbech commented 2 years ago

For anyone that can't wait I have created a chrome extension (https://github.com/mrbech/hackage-synopsis-search-hider) that applies the fix from https://github.com/haskell/haddock/pull/1486 to hackage.haskell.org