Skip to content

Load avatar in header via PHP#7749

Merged
MorrisJobke merged 2 commits intomasterfrom
load_avatar_header_via_php
Oct 23, 2014
Merged

Load avatar in header via PHP#7749
MorrisJobke merged 2 commits intomasterfrom
load_avatar_header_via_php

Conversation

@Niduroki
Copy link
Copy Markdown
Member

Load the avatar in the header via PHP instead of JS.

I can't really test this, because I get #7707 when I try to upload an avatar and thus can't upload avatars …

@owncloud/designers @jbtbnl

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Div is not allowed to be in the body of span, I suppose you can change the span with id "expand" to a div to solve this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't introduced by you but better solve it now :)

@blizzz
Copy link
Copy Markdown
Contributor

blizzz commented Apr 4, 2014

@Kondou-ger any update?

@Niduroki
Copy link
Copy Markdown
Member Author

Niduroki commented Apr 5, 2014

@blizzz I'm still wating for @LukasReschke's opinion to escaping this, and another string. But looks like he forgot me 😄

@LukasReschke
Copy link
Copy Markdown
Member

Nah, not forgotten. Just a lot of other stuff to do ;-)

@MorrisJobke
Copy link
Copy Markdown
Contributor

State of this?

@Niduroki
Copy link
Copy Markdown
Member Author

Niduroki commented Jun 2, 2014

#7749 (comment) is the state of this

@MorrisJobke
Copy link
Copy Markdown
Contributor

@LukasReschke Pingeling.

@Niduroki Niduroki force-pushed the load_avatar_header_via_php branch from 9f11e5b to c2fe2da Compare August 30, 2014 13:43
@LukasReschke
Copy link
Copy Markdown
Member

Change to p(), use the UID instead of the displayname and take a look at @jbtbnl's comment then I'm fine with it.

@Niduroki
Copy link
Copy Markdown
Member Author

@LukasReschke since you're not answering me in IRC to do this sort-of confidential I'll just spit this out (it's pretty specific – so if anyone can actually abuse this, good job …), regarding the "other string" from #7749 (comment).

How about escaping this: https://github.com/owncloud/core/blob/master/core/js/jquery.avatar.js#L74 ? LDAP Names that have a ?, # or & may screw this one up. How should we escape this? Is there a p() equivalent in JS?

@LukasReschke
Copy link
Copy Markdown
Member

urlencode - but this us not exploitable there since the username !== displayname

@Niduroki
Copy link
Copy Markdown
Member Author

Finally worked on this.
@LukasReschke @jbtbnl please re-review.

@ghost
Copy link
Copy Markdown

ghost commented Oct 19, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/705/
🚀 Test PASSed. 🚀

kondou and others added 2 commits October 23, 2014 23:17
@MorrisJobke MorrisJobke force-pushed the load_avatar_header_via_php branch from 9287c34 to a10b255 Compare October 23, 2014 21:52
@MorrisJobke
Copy link
Copy Markdown
Contributor

I squashed your commits and added the avatardiv-shown class, because this causes the display name to hide on small screens.

Works 👍

@MorrisJobke
Copy link
Copy Markdown
Contributor

@wakeup @luckydonald @miicha @danbartram @oparoz Feel free to review this. Just check out this branch and set an avatar for the user in the personal settings. then check if it appears in the header and the display name hides on small screens.

@scrutinizer-notifier
Copy link
Copy Markdown

The inspection completed: 2 new issues, 1 updated code elements

@danbartram
Copy link
Copy Markdown
Contributor

This works for me.

👍

@ghost
Copy link
Copy Markdown

ghost commented Oct 23, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/1116/
🚀 Test PASSed. 🚀

MorrisJobke added a commit that referenced this pull request Oct 23, 2014
@MorrisJobke MorrisJobke merged commit 4b5a387 into master Oct 23, 2014
@MorrisJobke MorrisJobke deleted the load_avatar_header_via_php branch October 23, 2014 23:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants