Confirm that clicking the first (external) link (called MusicBrainz’s community) does open a new tab
Close this new tab (back to the post)
Click the pencil icon (post modifications history)
Click that same external link in the history pop up panel
Instead of opening in a new tab, this external link opens in the current tab.
Disclaimer: this topic is not a duplicate of that post. That former post was indeed describing the same problem but the topic then diverged to fixing the starred () problem, which own topics got erased two times by mistake.
So, thanks for reading it, I tried to best describe a step by step explanation, as my English is often ambiguous otherwise, and as it was helpfully suggested by @zogstrip.
Hi @Mittineague, you should have noticed, at step 3., that this agnostic link bug has been fixed already.
It’s strange BTW, I have just noticed that now all internal links contained in posts (like this step 3. link above) are considered external and open in a new tab, now. This only happens in Opera 12, and as it is not supported by Discourse, it’s kind of irrelevant… But still worth of note as, before 22/4/2016, this problem did not occur.
Post links also do not open in a new tab from a user’s profile page. Go to this post: Search within topic is omitting results and click the first link, which will open in a new tab as expected.
Now go to my profile page, find that post and click the link. It opens in the same tab.
(I have no idea whether or not this is considered a bug; I’m simply reporting facts.)
I am not so sure that clicking links in post history (my case) or in user last posts (@TechnoBear case) are that much of edge cases…
And in both cases I would consider them links within a post, by the way.
So this change would probably be good but would not really fix the reported case(s).
Before acccepting my PR here are a couple of things for the Discourse Team to take into account (that I was too tired to write last night).
My PR copied some of the work that is in ClickTrack because integating ClickTrack either meant, changing the DOM output of user-stream to mimic topic-view or update ClickTrack to work with topic-view, user-stream, and whatever the edit history is using. That way we have one spot dictating how/when to open links in a new window.
So with that said, if I were to improve ClickTrack, which way do you want me to go with it? I don’t think we can transform Edit History to mimic topic-view, so that leads me towards altering ClickTrack so it understands what is requesting it (an optional second param that defaults to topic, but also accepts edit-history and profile).
Then transform some of the calls to work for those other views.
Lastly, do we want to call the click tracking URL so that Discourse logs the click when it occurs from edit history or the profile page? Today, it does not track those clicks, so this would be a change to existing behavior.
No, the server needs a post_id op topic_id. The initial thought required the link is inside the topic which reflects in topic map. Without the parameters, you won’t be redirected anyway.
IMO, improvements basically can be done by:
Change nothing but regulate the option text.
Separate opening/tracking from ClickTrack and inject opening lib everywhere (History, Profile)
That is sort of where my initial thoughts with the first outline were going. As if I send an optional parameter I can skip the actual link tracking logic for all uses except those coming from a topic.
Then I don’t have to worry about the parameters needed. But if Discourse wants those tracked I don’t think it will be hard to get that data either from the profile/history pages.
The main point was my current PR duplicates efforts, of which I’m not a big fan of but it shows it can be done.
This one altered the click-track library to receive an optional second parameter (per my outline above).
I also added qunit tests for the Profile Page. I added them as a seperate file because it seemed the cleaner way to add the tests, but if there is a more preferred way, I’m open to suggestions.
I’m going to work on the Edit History either later this week, or maybe next week. But this at least solves the Profile Page.
I’m not sure how that would work, as it seems you have to force links to run through an onclick event, and ember likes to attach its own for handling actions… and you obviously don’t want to take over that, as it could break things like the topic list go to first/last post and other operations that popup little modals on click, share links, etc.
Plus middle clicking the edit icon, shouldn’t open in a new tab, nor should the flag icon, so I feel like we’d have to make an exclusion list. (granted those are buttons, so the onclick event could be specific to a), but there are a links that are opening little modals/UIs that shouldn’t open in a new window
Keep in mind, I’m not really duplicating anymore. My new PR extends the existing ClickTrack functionality to work with the Profile Page and not blow up trying to log the click details.
So the only thing I duplicated, is I told the Profile Page to call the ClickTrack logic, which is an on event handler. (so now an on event handler exists in the topic-stream and the user-stream that call ClickTrack)
I also added tests against the Profile Page so it shouldn’t regress in future versions. I plan to do the same thing with the Edit History dialog when I get a chance (extend ClickTrack to include it and add an on event to the Edit History Modal dialog)
We also need to be mindful of plugins. As overwriting what their intent could be problematic.
What exactly is the difference in the DOM that prevents it from working? I am curious about that. I definitely don’t like duplicating all the code as you’ve suspected
I don’t see why not. Edit history is not very interesting but user profile clicks are definitely valid clicks.
Well the DOM is entirely different between the two pages. The profiles are using the excerpt and are not a full article which is what ClickTrack was expecting. As such, ClickTrack isn’t capable of finding postId or topicId. I’m not saying it isn’t possible, but none of those data attributes are on the excerpts shown on the Profile pages, so that view would need to be altered (not a big issue).
More importantly, though, again, this isn’t duplicating anything anymore. I just extended ClickTrack to work even when the postId/topicId are not present, which permits it to work on the Profile Page and will permit it to work with Edit History once I tell that logic to call ClickTrack for handing the links found in the post. By extending ClickTrack to not fail when postId/topicId aren’t present, it gives a way for Discourse to use it as “the” click handler for all things post related.
Disclaimer: this topic is not a duplicate of that post. That former post was indeed describing the same problem but the topic then diverged to fixing the starred (
) problem, which own topics got erased two times by mistake.
)