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

RESOLVED FIXED

Status

Webmaker
Thimble
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: fuzzyfox, Assigned: pomax)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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).

Updated

5 years ago
Assignee: nobody → pomax
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
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.
(Reporter)

Comment 2

5 years ago
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
(Assignee)

Comment 3

5 years ago
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.
(Assignee)

Comment 4

4 years ago
unassigning pending reprioritisation
Assignee: pomax → nobody
(Assignee)

Comment 5

4 years ago
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 '#'.
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.
(Assignee)

Comment 8

4 years ago
that would also work. I can flip the code around to be basetagless.
(Assignee)

Comment 9

4 years ago
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
Attachment #804585 - Flags: review?(scott) → review-
(Assignee)

Comment 10

4 years ago
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.
Attachment #804585 - Flags: review- → review?(scott)
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.
Attachment #804585 - Flags: review?(scott) → review+
(Assignee)

Updated

4 years ago
Assignee: nobody → pomax

Comment 12

4 years ago
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
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.