Closed Bug 888254 Opened 12 years ago Closed 12 years ago

Links in embedded Thimble pages navigate in-frame (causing multiple make details bars, etc)

Categories

(Webmaker Graveyard :: Thimble, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: laura, Assigned: michiel)

Details

Attachments

(1 file)

STR Go here: https://mozteach.makes.org/thimble/how-to-use-the-hacking-guides Click on "Online Storytelling", notice the second make details bar. Now click on "Spectrogram for Interactive Video", notice the third make details bar Should only display the Make Details for the Make the user is looking at. Shouldn't it also take the user to the URL of whatever it is they clicked on? If you follow these STR, look at the URL, it always stays https://mozteach.makes.org/thimble/how-to-use-the-hacking-guides
morphed to its relevant Thimble bug - we can solve this by giving the embedded page a <base> element that says that all URLs must open in a new tab/window. We might also be able to set this on the iframe rather than its content, so that pages won't need this element embedded, but that depends on whether we can access the iframe's contentWindow/contentDocument. We probably can, since we control the iframe's position on the page.
Component: webmaker.org → Thimble
Summary: Make Details on child pages and direct linking → Links in embedded Thimble pages navigate in-frame (causing multiple make details bars, etc)
also note this requires the "base" tag to be in the bleach.js whitelist, with the "href" and "target" attributes whitelisted for it, too.
Assignee: nobody → pomax
Attachment #768961 - Flags: review?(scott)
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 I'm fine with this. Looks like jbuck has some comments though. Should probably run this by him too.
Attachment #768961 - Flags: review?(scott) → review+
Attachment #768961 - Flags: review+ → review?(jon)
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 Well I'd really like you to use res.render() instead of rendering that template through the nunjucks API...
Attachment #768961 - Flags: review?(jon) → review+
I'd much rather have the entire thing as a single nunjucks-with-inclusions file. res.render() hides too much of what it's using for its variable instantiation, and the way we're using it here, has nothing to do with the response at all, making it an inappropriate API.
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 had some odd git rebase artifacts going on, I'm pretty sure the code's the same as pre-rebase now, but I'd like to be sure here.
Attachment #768961 - Flags: review+ → review?(scott)
Attachment #768961 - Flags: review?(scott) → review+
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 Actually hold up. Not working for me anymore.
Attachment #768961 - Flags: review+ → review?(scott)
did this work again after clearing the dbs?
Attachment #768961 - Flags: review?(scott) → review?(chris)
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 actually, there's some errors left that need fixing. removing outstanding review requests.
Attachment #768961 - Flags: review?(chris) → review-
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 fixed the bad filename. According to jbuck the errors yesterday came from a bad commit on the makeapi, so this should be testable either with an updated, fixed makeapi, or by revert to prior to the bad commit.
Attachment #768961 - Flags: review- → review?(scott)
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 One line comment. Should be good after a rebase and a fixing the comment.
Attachment #768961 - Flags: review?(scott) → review-
Scratch that, two comments.
Comment on attachment 768961 [details] [review] https://github.com/mozilla/thimble.webmaker.org/pull/165 rebased, base tag placement fixed, too. You're a lifesaver overhere.
Attachment #768961 - Flags: review- → review?(scott)
Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org https://github.com/mozilla/thimble.webmaker.org/commit/a1c62c2975426cacfac56eb37cc3cbf55ee21a20 Merge pull request #165 from Pomax/bug888254 Bug888254 - made links use _blank rather than loading in the iframe embed.
and there was much rejoicing.
Status: NEW → RESOLVED
Closed: 12 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.

Attachment

General

Creator:
Created:
Updated:
Size: