897430 - anchor links `href="#someid"` within thimble makes being prefixed with S3 URL to make.

Status: RESOLVED FIXED

(Webmaker Graveyard :: Thimble, defect)

Description

[William Duyck [:fuzzyfox]]

Anchor links `href="#someid"` within thimble makes being prefixed with S3 URL to make.

This causes them to break the iframe that the makes are shown in on webmaker.org, as well as removes the ability to use some cool CSS stuff like :target for galleries, etc...

It also prevents makes from linking to elsewhere within itself (which might be nice for large makes).

Comment 1

[Pomax]

do you have an example make for debug-prodding? Far as I recall the <base> tag we introduced only adds a default "_blank" target, so it shouldn't actually turn links into fully qualified links.

Comment 2

[William Duyck [:fuzzyfox]]

Working example:      http://jsfiddle.net/jkTRB/
Same code in thimble: https://fuzzyfox.makes.org/thimble/the-interview-buggy

It seems codepen.io has the same issue  :P Time to drop them a bug too.

My use case is using css :target, but the issue seems to be with anchors becoming fully qualified links, and not with the way I'm using :target

Comment 3

[Pomax]

Ah. This is actually an interesting spec issue. The <base> tag tells all links to open in a specific target, which means it has to fully resolve them to open, but #fragment navigation is not a request for a new resource, so <a> elements shouldn't trigger on a target at all. This actually looks very much like a browser bug due to a missing use case when they were coding it (FF and Chrome both do this). You can override the <base> behaviour by adding target="_self" do your in-page links, but this might be worth filing on the various browser bug trackers, too.

Comment 4

[Pomax]

unassigning pending reprioritisation

Comment 5

[Pomax]

We can probably solve this by, in the finalisedHTML, adding not just the base tag, but also a mutation observer that automatically tacks 'target="_self"' to any <a> element with an href that starts with '#'.

Comment 6

[Pomax]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/216

Comment 7

[Scott [:thecount] Downe]

A pretty gross problem.

Your solution isn't bad considering the problem.

My only thoughts are:

If we removed the base tag, this would go away, yes? Is the base tag worth it? Seems like if it causes issues, we should consider removing it. Then we could add something like what you just added, but instead of adding target=_self to links with a #, we could add target=_blank to links without a target or #?

Would that be better, if it works?

The less we need to change the user's project, the better. This problem exists because of the fact it lives in a details page.

Comment 8

[Pomax]

that would also work. I can flip the code around to be basetagless.

Comment 9

[Pomax]

Comment on attachment 804585 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/216

r- in while I flip the switch to solving the problem from the other direction

Comment 10

[Pomax]

Comment on attachment 804585 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/216

new implementation, without <base> and with target="_blank" for all external links instead.

Comment 11

[Scott [:thecount] Downe]

Comment on attachment 804585 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/216

R+, just change the functions to include function name(){}

Up to you if you want me to take a look again after that change.

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org

https://github.com/mozilla/thimble.webmaker.org/commit/c326a5af3e5f67b0194bde4e7550920421390405
Merge pull request #216 from Pomax/bug897430

forcing local links to stay local, despite base target=_blank