Bugfix outlink encode issue #7786
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @saleemkce thanks for the Pull request. Click on "Files" you will see your commit for this pull request, as you can see it includes your other commits from other pull requests. maybe you created your change in the same branch? if you create a different branch for each PR it should help you. also check out this guide: http://developer.piwik.org/guides/contributing-to-piwik-core#hacking-piwik let us know how you go, or if you have any question! |
|
I find some recurring merge-commit issues. For now, I have deleted all inconsistent pull requests for this issue. I will do this later. I feel quite sorry and embarrassed about not being able to give clean PR now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for issue #5014
It initially took almost one and a half hours to debug the issue and finally got the bug. I wanted to fix this issue because I have had great experience working with urlencode/decode functions in OSIpage in the past so I thought it would be easy for me to debug the issue.
Misconceptions assumed when debuggin:
1, Tried to add meta charset="UTF-8" to head section in HTML page. Didn't help
2, Tried to add script type="text/javascript" charset="text/html;UTF-8" in Piwik script inclusion tag in the hope that it will force page to accept any language character. Didn't help.
3. Inspected Jquery's anchor tag that was bound to DOM when user clicked the outbound link. The dom's href tag was perfect, it contained correct URL with another garbled URL in DOM node (this was updated by our own code)
4. So dug all the related link processing functions and found the line responsible for creating the garbled URL,
and replaced with,
5, Thus, all links are now correctly stored in DB and in admin pages.
Some screenshots to enjoy (I used OSIpage developement code to reproduce outbound links),
[end user page when clicking outbound links, check correct Arabic URL in console]
[Correct URL (Arabic, Chinese) display in admin pages]
One consideration
when committing, I found warning: LF will be replaced by CRLF in piwik.js.. I don't have clear idea how to make sure correct line ending should be propagated to remote repo. Please cross-check it.
Please comment if anything is wrong.