Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Me: Sidebar: Decode entities for User's display name on /me profile sidebar #625

Merged
merged 1 commit into from
Nov 24, 2015

Conversation

oskosk
Copy link
Contributor

@oskosk oskosk commented Nov 24, 2015

Display names that used a special char, for example, an ampersand were shown like & on the h2 that shows the display name under the gravatar.

It used to look like this
Entities not decoded on /me display name

Now it shows just the ampersand.

cc @ebinnion

@oskosk oskosk added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account. Sidebar labels Nov 24, 2015
@artpi
Copy link
Contributor

artpi commented Nov 24, 2015

Tested, looking at code

@artpi
Copy link
Contributor

artpi commented Nov 24, 2015

🚢

@artpi artpi added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 24, 2015
@ebinnion
Copy link
Contributor

LGTM.

The only feedback I have is that we should probably squash the commits here since the second commit is a fix for a whitespace issue that was introduced in the first commit.

@oskosk oskosk force-pushed the fix/me-display_name-not-decoded branch from 4685bd4 to 498b4f8 Compare November 24, 2015 17:16
@oskosk
Copy link
Contributor Author

oskosk commented Nov 24, 2015

Done. Squashed.

ebinnion added a commit that referenced this pull request Nov 24, 2015
Me: Sidebar: Decode entities for User's display name on /me profile sidebar
@ebinnion ebinnion merged commit fb3071b into master Nov 24, 2015
@ebinnion ebinnion deleted the fix/me-display_name-not-decoded branch November 24, 2015 17:46
@ebinnion
Copy link
Contributor

Going to go ahead and merge as this solves the issue with escaping entities in the sidebar.

@oskosk and I discussed different options for also escaping values that are displayed in the inputs throughout /me.

I suggested that we could potentially decode the HTML entities in the valueLink method within form-base. The only issue I see with this is that we should preferably escape as we are passing the value in to the FormInput, FormTextarea, etc.

@oskosk mentioned that he will create another PR to address the display of HTML entities within form fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants