PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Discuss requests for comments/changes posted in the Issue Tracker for the development of phpBB. Upcoming releases are 3.2/Rhea and 3.3.
Post Reply
User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Tue Jun 09, 2015 12:24 pm

Hello,

For a new type of profile field that I am working on ("Image Upload Profile Field"), I need to upload files from the local user storage.

To use it, the HTML form must include the correct "enctype" clause.

In certain places, it is already present, hardcoded (acp_users_avatar.html) or in a template variable (ucp_profile_profile_info.html).

But there are other places where none of these options is available: acp_users_profile.html, acp_profile.html.

The only viable option here is to modify the ACTION URL template variable to include enctype: ugly, dangerous and incorrect.

So, the only possibility is to fix the core. Given that this is not a request for an Event, I though we should discuss it here. I will create the patch, with whatever the resolution we take here is.

In my opinion, hardcoding in the ACP is not that bad, and it is much easier and cleaner in all respects. For the UCP, the variable option is already present. So that is my preference, to get started with the discussion.

Regards,
-javiexin

The ticket in the tracker: https://tracker.phpbb.com/browse/PHPBB3-13934

User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Wed Jun 10, 2015 12:21 pm

No opinions, so I guess nobody cares which way we go. I will propose the patch with the solution I indicated above: adding "enctype" hardcoded to the relevant forms in the ACP.

User avatar
Pony99CA
Registered User
Posts: 986
Joined: Sun Feb 08, 2009 2:35 am
Location: Hollister, CA
Contact:

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by Pony99CA » Thu Jun 11, 2015 1:47 am

I can't help with choosing, but I'm curious if you're basically implementing the image Custom Profile Field type that I asked for in the New Custom Profile Field Types RFC.

If so, please also allow the options that I mentioned. I would also recommend allowing both linked and uploaded images.

Steve
Silicon Valley Pocket PC (http://www.svpocketpc.com)
Creator of manage_bots and spoof_user (ask me)
Need hosting for a small forum with full cPanel & MySQL access? Contact me or PM me.

User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Thu Jun 11, 2015 7:53 am

Not quite the same, but exactly in that direction, and mostly applicable. I had not seen this post, thank you.

I am implementing several CPFs, and specifically, an Image Upload and an Image Selector CPFs. This need is for the Image Upload CPF.
  • Image: Allow displaying an image (with user-specified sizing and ALT text -- possibly with some site-specified maximums). Allowing a larger-sized image (with possible lightbox display) would be even better.
Image Upload will not support Alt text specification: no place to store it, and would be overkill to add another field just for this. Each CPF of this type will specify min & max dimensions, and max weight. It will generate <img></img> wrapped URL for display, along with clean URL to the image, so that you may adapt the display.
Image Selector will allow to select an image from all the ones available in a specified folder (per CPF defined). Same display options.

The extension will include another "visibility" option, that basically disables display within the custom profile loops, yet keeps the individual template vars, so that the display implementation may be fully customized (adding lightbox or other effects).

But I am finding significant issues with the implementation. Anyhow, progressing...

-javiexin

User avatar
Mess
Registered User
Posts: 197
Joined: Wed Jun 13, 2012 10:14 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by Mess » Fri Jun 12, 2015 10:09 am

Unfortunately I do not have anything meaningful to contribute with.
I just want to tank you for taking this on. I feel CPFs are very important and I'm happy to see you expand on existing features.

User avatar
Pony99CA
Registered User
Posts: 986
Joined: Sun Feb 08, 2009 2:35 am
Location: Hollister, CA
Contact:

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by Pony99CA » Sat Jun 13, 2015 12:11 am

javiexin wrote:Not quite the same, but exactly in that direction, and mostly applicable. I had not seen this post, thank you.

I am implementing several CPFs, and specifically, an Image Upload and an Image Selector CPFs. This need is for the Image Upload CPF.
  • Image: Allow displaying an image (with user-specified sizing and ALT text -- possibly with some site-specified maximums). Allowing a larger-sized image (with possible lightbox display) would be even better.
Image Upload will not support Alt text specification: no place to store it, and would be overkill to add another field just for this. Each CPF of this type will specify min & max dimensions, and max weight. It will generate <img></img> wrapped URL for display, along with clean URL to the image, so that you may adapt the display.
Image Selector will allow to select an image from all the ones available in a specified folder (per CPF defined). Same display options.
Regarding ALT text, this is for accessibility (allowing screen readers to read what the image shows). How is it overkill?

As for Image Selector, can't you just have one Image CPF and allow selecting upload, URL or selected folder as part of the CPF options? That would be similar to how avatars work.

Steve
Silicon Valley Pocket PC (http://www.svpocketpc.com)
Creator of manage_bots and spoof_user (ask me)
Need hosting for a small forum with full cPanel & MySQL access? Contact me or PM me.

User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Sat Jun 13, 2015 10:47 am

Pony99CA wrote:Regarding ALT text, this is for accessibility (allowing screen readers to read what the image shows). How is it overkill?
For me, overkill is adding an extra database field for storing Alt text for one CPF. The behavior is going to be similar to that of avatars, with the same alt text for all instances of the same CPF.
Pony99CA wrote:As for Image Selector, can't you just have one Image CPF and allow selecting upload, URL or selected folder as part of the CPF options? That would be similar to how avatars work
No, I am looking into different use cases. And currently I am planning no URL images.

Regards,
-javiexin

User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Tue Jun 30, 2015 12:46 pm

Patch submitted as PR: https://github.com/phpbb/phpbb/pull/3733

Finally, all cases (ACP and UCP) are using a template variable.

User avatar
javiexin
Registered User
Posts: 90
Joined: Thu Dec 22, 2011 10:04 am

Re: PHPBB3-13934 - Enctype clause for forms may be needed for profile fields

Post by javiexin » Tue Jun 30, 2015 10:50 pm

I need opinions regarding the discussion about this proposed change at:
https://github.com/phpbb/phpbb/pull/3733

Please, read there and post your opinion.
-javiexin

Post Reply