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

Create Pages/Menus > Enter Page Data and Saved Username is in Menu Name #3131

Closed
MikeyGMT opened this issue May 16, 2018 · 20 comments
Closed
Labels
core: pages/menus type: bug A problem that should not be happening type: enhancement An improvement or new feature request
Milestone

Comments

@MikeyGMT
Copy link
Contributor

Annoying little browser issue. When creating and editing Pages, in chrome, at least, the username appears in the Menu Name. I'll check in other browsers.

Makes it a wee bit clunky for new users...
image

image

Or worse creates a new menu/page hybrid instead of page.
image

This is the inspect source:

<input type="text" name="menu_name" value="XXX_username_XXX" maxlength="255" id="menu-name" class="tbox form-control ui-state-valid" pattern="^[a-z0-9-]*" data-original-title="" title="" aria-describedby="tooltip991890" autocomplete="off">

Really this should be a fresh issue. But from a usability point of view, I would like to see Create/Edit Pages and Menus split into separate functions, as they are in the new user's head different, well, to me as well. If I am showing a new user how to edit / create pages, then have to explain that you ignore the menu bit, you get a blank face, and they say ... "Oh I don't know, I am not very technical, can I leave it to you?". Which is what I want to avoid, it's their site to update. If you see what I mean.

I would not know where to begin, to split them myself, so won't attempt it. :)

@MikeyGMT
Copy link
Contributor Author

MikeyGMT commented Jun 13, 2018

Seems like the issue is not new. https://productforums.google.com/forum/#!topic/chrome/f6zhGC8lVw4 There is a suggestion to add to form tag autocomplete="off"
Edit: But we have that set on the form and the field. Hmmm.
Any thoughts?

@CaMer0n
Copy link
Member

CaMer0n commented Jun 13, 2018

Thanks @MikeyGMT ! Yeah, perhaps we can add autocomplete="off" by default on all forms.

@MikeyGMT
Copy link
Contributor Author

Seems the fix is autocomplete="false" I am just testing now.

@MikeyGMT
Copy link
Contributor Author

Hi Cameron!
OK, here there's lots of suggestions https://stackoverflow.com/questions/15738259/disabling-chrome-autofill/15917221#15917221 - seems issue keeps coming back.

I have changed to false in the form handler, but did not work. I am looking further into it.

@MikeyGMT
Copy link
Contributor Author

But WC3, IE and FF require On/Off https://bugs.chromium.org/p/chromium/issues/detail?id=468153#hc41
Google are overriding Off/False, as they know best.
My problem is they are putting my username in the menu name - I think because it looks in at a form for a password input and that assumes there will be a username.
And they have instead openned a new bug, asking us to justify our usage of autocomplete=off https://bugs.chromium.org/p/chromium/issues/detail?id=587466

@MikeyGMT
Copy link
Contributor Author

I have added a comment to the latter chrome bug. I guess the workaround is drop or rename the page password field or recommend folk not to use chrome.

@MikeyGMT
Copy link
Contributor Author

Screenshot of the Page Password option, for the Chrome Team to view
image

@MikeyGMT
Copy link
Contributor Author

I tested disabling the page password and chrome still autofills. FF is working correctly. I guess we can close this unless anyone has any bright ideas.

@MikeyGMT
Copy link
Contributor Author

MikeyGMT commented Jun 13, 2018

I've read through this https://stackoverflow.com/questions/7223168/how-to-trigger-autofill-in-google-chrome/9795126#9795126 and the links within it, and there's lots of clever stuff to identify fields which are potentially autofillable, but I am not fully understanding how we can specifically prevent autofill. https://www.w3.org/TR/2010/WD-html5-20101019/the-input-element.html#concept-input-mutable for example, isn't written in the clearest language!

What I got from it was probably a long shot, but maybe label could help us. Our code does not have label for input fields, instead we have a span. More importantly, labels are essential for screen readers used by visually impaired users.
Here is the browser code we send. How easy is it to convert the field title e.g. "Menu Name" from span to label?

<span>Menu Name</span>
<input name="menu_name" value="" maxlength="255" id="menu-name" class="tbox form-control ui-state-valid" pattern="^[a-z0-9-]*" autocomplete="off" data-original-title="" title="" type="text"> <div class="field-help" style="display: none;">Will be listed in the Menu-Manager under this name or may be called using {CMENU=name} in your theme. Must use ASCII characters only and be all lowercase.</div>

This is the array for menu.
'menu_name' => array('title'=> CUSLAN_64, 'nolist'=>true, 'tab' => 2, 'type' => 'text', 'width' => 'auto', 'autocomplete' => 'off', "help"=>"Will be listed in the Menu-Manager under this name or may be called using {CMENU=name} in your theme. Must use ASCII characters only and be all lowercase."),
Hence does the title element needs to be returned as a label tag?

@Moc Moc added type: bug A problem that should not be happening type: enhancement An improvement or new feature request labels Jun 23, 2018
@Moc Moc added this to the e107 2.1.9 milestone Jun 23, 2018
@CaMer0n CaMer0n modified the milestones: e107 2.1.9, e107 2.2.0 Aug 22, 2018
@CaMer0n
Copy link
Member

CaMer0n commented Nov 12, 2018

@MikeyGMT Does changing span to label fix the problem for you?

@MikeyGMT
Copy link
Contributor Author

Hi @CaMer0n,
I change form_handler.php line 6962 from

$leftCell = $required."<span{$required_class}>".defset(vartrue($att['title']), vartrue($att['title']))."</span>".$label;
To use label ...

$leftCell = $required."<label{$required_class}>".defset(vartrue($att['title']), vartrue($att['title']))."</label>".$label;
Still the same - in the same session! However, successful when switched to incognito for a quick/dirty test without clearing browser cache. The username has gone from the menu_name field! I need to clear cache etc to confirm - will do more testing over the weekend.

It is quite a big impact change, a few folk need to test thoroughly. Maybe others can test by making that change locally? If it really does work, it would improve screen-reader compatibility enormously across all e107 websites.
Thanks
Mike

@MikeyGMT
Copy link
Contributor Author

It did not fix it, it is still convinced this is a username field in chrome. But I think it may work if we add the for=menu_name but I am not sure how to do that.

@MikeyGMT
Copy link
Contributor Author

OK, so added for=".$keyName."

$leftCell = $required."<label{$required_class} for=".$keyName.">".defset(vartrue($att['title']), vartrue($att['title']))."</label>".$label;

Rendered code is correct, but still chrome is assuming menu_name is a variant of user_name, because there is a password field.

I tried adding a user_name tab option field in cpage to fool it, but nope, no luck.

@MikeyGMT
Copy link
Contributor Author

OK, so I have found a workaround, change the lan CUSLAN_64 to Menu Code or menu_code (in fact, it is more accurate to call it "menu_code") - and the field tab option name to menu_code, everything else is set back to today's git. Menu Name somehow get's attributed to user_name. Bizarre, you should read the google bugs, people are getting (quite rightly) very irritated with the issue.

define("CUSLAN_64", "Menu Code");

'menu_code' => array('title'=> CUSLAN_64, 'tab' => 2, 'type' => 'text', 'width' => 'auto','nolist'=>true, "help"=>"Will be listed in the Menu-Manager under this menu_code or may be called using {CMENU=name} in your theme. Must use ASCII characters only and be all lowercase."),
No idea if that change will break anything elsewhere!

@MikeyGMT
Copy link
Contributor Author

Oh dear, it breaks all over the place.

Will need menu_name replacing in 23 files! Who knows in plugins and themes. This is not a valid workaround.

"You must enter either a page title or a menu name."

Quick fix tried was changing from text to textarea, not working. Played around with other settings, no success. Nearest fix seems to be to rename menu_name to menu_code.

@MikeyGMT
Copy link
Contributor Author

Seems to me change the name of the menu_name field or maybe consider this https://github.com/terrylinooo/jquery.disableAutoFill

@CaMer0n
Copy link
Member

CaMer0n commented Nov 19, 2018

Thanks Mikey! Or maybe chrome could fix their bug. smh.

@Moc
Copy link
Member

Moc commented Oct 30, 2019

It seems this tip worked: https://gist.github.com/niksumeiko/360164708c3b326bd1c8#gistcomment-3004386

Pushed it, and tested on the latest Chrome and Firefox.

@Jimmi08 could you test please?

Moc added a commit that referenced this issue Oct 30, 2019
Moc added a commit that referenced this issue Oct 30, 2019
@MikeyGMT
Copy link
Contributor Author

Looking good!

@Jimmi08
Copy link
Contributor

Jimmi08 commented Oct 31, 2019

I looks fixed. PHP 7.3 Excellent job. Thank you.

@Moc Moc closed this as completed Nov 27, 2019
@Moc Moc modified the milestones: Future, e107 2.3.0 Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core: pages/menus type: bug A problem that should not be happening type: enhancement An improvement or new feature request
Projects
None yet
Development

No branches or pull requests

4 participants