docsify icon indicating copy to clipboard operation
docsify copied to clipboard

fix: render image in absolute path

Open dandrade-meli opened this issue 3 years ago • 6 comments

Summary

This PR documents support for including images with absolute paths and fixes a small bug that made it impossible to use. This avoids the need to use ../../ to get to the root path. Enable to fix bugs #850, #415

Example of use:

![example-logo](:_images/example/example-logo.png)
![example-logo](//_images/example/example-logo.png)
docsify
├── _images
│   └── example
│       └── example-logo.png
└── example
    ├── _coverpage.md | ![example-logo](:_images/example/example-logo.png)
    ├── _sidebar.md
    └── README.md
_coverpage.md
_sidebar.md
index.html
README.md

in /core/render/compiler/image.js which is responsible for generating the html img, there is a check if the href is not absolute.

if (!isAbsolutePath(href)) {
   url = getPath(contentBase, getParentPath(router.getCurrentPath()), href);
}

the isAbsolutePath function uses the following regex to do this check

/(:|(\/{2}))/g

what this regex does: if the url contains : or //, then this path is absolute.

but when we used this strategy, adding the : or //, the image was not rendered because in the generated img tag the path kept the : or //

<img src=":_images/example/example-logo.png" data-origin=":_images/example/example-logo.png" alt="" title="">
<img src="//_images/example/example-logo.png" data-origin="//_images/example/example-logo.png" alt="" title="">

and to fix this bug I applied a simple replace to remove // and : by ``

<img src="_images/example/example-logo.png" data-origin="//_images/example/example-logo.png" alt="" title="">
<img src="_images/example/example-logo.png" data-origin="//_images/example/example-logo.png" alt="" title="">

What kind of change does this PR introduce?

Bugfix Feature Docs

For any code change,

  • [x] Related documentation has been updated if needed
  • [ ] Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

  • [ ] Yes
  • [x] No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

#850 #415

Tested in the following browsers:

  • [x] Chrome
  • [x] Firefox
  • [x] Safari
  • [ ] Edge
  • [ ] IE

dandrade-meli avatar Aug 23 '22 13:08 dandrade-meli

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
docsify-preview ✅ Ready (Inspect) Visit Preview Sep 1, 2022 at 9:48AM (UTC)

vercel[bot] avatar Aug 23 '22 13:08 vercel[bot]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e9fe6a93b420ba0ecc56e3b736ec63a9ba1a1653:

Sandbox Source
docsify-template Configuration

codesandbox-ci[bot] avatar Aug 23 '22 13:08 codesandbox-ci[bot]

Let's identify the behavior we want first without worrying about implementation details. I think we also need to consider if/how Docsify's relativePath configuration option weighs into this discussion.

Given the following Docsify site structure:

[Root]
└── dir
    └── FILE.md
_sidebar.md
index.html
README.md

I believe Docsify's HTML output for /dir/FILE.md image URLs should be as follows:

Markdown URL Proposed output
relativePath:false
Proposed output
relativePath:true
current output
/img.png /img.png dir/img.png /dir/img.png
img.png img.png dir/img.png /dir/img.png
../img.png ../img.png ../img.png /dir/../img.png
/dir/img.png /dir/img.png dir/img.png /dir/img.png
../dir/img.png ../dir/img.png dir/img.png /dir/../dir/img.png

The table can be a bit confusing but the rules required to match the proposed output are simple:

  1. When relativePath:false (default), do not modify the markdown URL
  2. When relativePath:true...
    • Make URLs relative to the file context if the URL resolves to target within the file context. If the URL resolves to a target outside of the file context, do not modify the URL
    • Output relative URLs instead of absolute URLs. This allows Docsify sites contained in subdirectories to function properly

Thoughts?

jhildenbiddle avatar Sep 09 '22 20:09 jhildenbiddle

Let's identify the behavior we want first without worrying about implementation details. I think we also need to consider if/how Docsify's relativePath configuration option weighs into this discussion.

Yea, If the relativepath config is the highest rule, what the users do on paths should follow this config, then things should work well. Unfortunately, users wanna out of the rule and they can manually set one or more specific paths to absolute. So, about this solution is based on : A. Force user abide by the rules of the game. B. Change the rule and left a back door.

Personally, I think it is fine to give users more sugars and tricks if the change would not break a big thing.

Koooooo-7 avatar Sep 10 '22 16:09 Koooooo-7

Let's identify the behavior we want first without worrying about implementation details. I think we also need to consider if/how Docsify's relativePath configuration option weighs into this discussion.

Yea, If the relativepath config is the highest rule, what the users do on paths should follow this config, then things should work well. Unfortunately, users wanna out of the rule and they can manually set one or more specific paths to absolute. So, about this solution is based on : A. Force user abide by the rules of the game. B. Change the rule and left a back door.

Personally, I think it is fine to give users more sugars and tricks if the change would not break a big thing.

I believe the prefix :absolute idea adds the possibility without breaking the default behavior.

dandrade-meli avatar Sep 15 '22 12:09 dandrade-meli

TL;DR:

  • Review #1891

  • Rename :absolute to :basepath and allow it to accept a value:

    ![](image.png ':basepath=/assets/')
    
  • Let's determine if/how/when we fix the other path-relative issues


I believe the prefix :absolute idea adds the possibility without breaking the default behavior.

It could, but...

  1. Docsify's path handling is problematic and/or broken in multiple scenarios, not just this one. The issue this PR attempts to solve is just one symptom of larger problems that need to be addressed.

    I don't mean that to sound critical of this PR as I appreciate the effort and recognize the need for a fix. My intention is only to call attention to the larger problems first so we can determine the best path forward.

    I've created a separate issue that explains the path-related issues in more detail and provides solutions here: #1891. It can be a lot to digest, but the summary is simply that if we make path handling 1) consistent and 2) behave as expected (based on how they work everywhere else) by default, additional configuration options and custom markdown syntax will not be needed by the overwhelming majority of users.

    For perspective, here is a table demonstrating the current path-related issues:

    CleanShot 2022-09-19 at 15 08 31@2x

  2. Images are not the only resource that Docsify makes inaccessible via absolute path. The same issue affects embedded content (path is relative to the page route) and markdown pages in a nested site (path is relative to index.html). If we are going to add an :absolute attribute for image, we should do the same for other elements as well.

  3. The preferred behavior for an :absolute attribute can change based on the scenario. For example, assuming :absolute means "relative to the top-level of the domain or directory hierarchy" works here:

    [Root]
    ├─ assets
    │  └─ image.png
    ├─ dir
    │  └─ page.md
    ├─ index.html
    └─ README.md
    
    <!-- Works on README.md or page.md -->
    
    ![](/assets/image.png ':absolute')
    
    <img src="/assets/image.png">
    

    But the same behavior does not work here:

    [Root]
    └─ docs
       ├─ assets
       │  └─ image.png
       ├─ dir
       │  └─ page.md
       ├─ index.html
       └─ README.md
    

    Users could add /docs/ to their absolute path:

    <!-- Works on README.md or page.md -->
    
    ![](/docs/assets/image.png ':absolute')
    
    <img src="/docs/assets/image.png">
    

    But it is not unreasonable to assume or want "absolute" to mean "relative to the top-level of my site". In order to accomodate both scenarios, the attribute should accept a value. Renaming the attribute to :basepath makes sense to me since this is the purpose served by the attribute:

    ![](image.png ':basepath=/assets/')
    ![](image.png ':basepath=/assets/docs')
    
    <img src="/assets/image.png">
    <img src="/docs/assets/image.png">
    

@Koooooo-7: Personally, I think it is fine to give users more sugars and tricks if the change would not break a big thing.

I believe the goal should be for Docsify to be as intuitive as possible. Docsify's path handling is not intuitive. This is the result of both bugs and questionable design decisions made long ago.

These issues aren't obvious or common because they only present themselves is less-common scenarios (which is why they've survived this long), but when they do present themselves it's a mess. Adding configuration options and custom markdown syntax to get out of the mess instead of just fixing the issues that cause the mess is just avoiding the problem and adding more noise, both of which make Docsify less intuitive and less enjoyable for users.

FWIW, I have no issues adding configuration options or custom markdown syntax to to make Docsify more capable or enjoyable to use. I'm okay adding an :absolute-like feature so long as long as it makes sense in the big picture (i.e., after we fix the larger issues).

jhildenbiddle avatar Sep 19 '22 20:09 jhildenbiddle

close via #1891. @dandrade-meli thx for your input ! If we finish the path issue, we could reopen it if needs.

Koooooo-7 avatar Oct 04 '22 07:10 Koooooo-7