Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

JHtmlBehavior::formvalidation() #1546

Closed
piotrmocko opened this issue Sep 24, 2012 · 24 comments
Closed

JHtmlBehavior::formvalidation() #1546

piotrmocko opened this issue Sep 24, 2012 · 24 comments

Comments

@piotrmocko
Copy link

JavaScript form validation does not work because mootools-more is not loaded.

file:
libraries/joomla/html/behavior.php
method:
formvalidation()
change:
self::framework();
to:
self::framework(true);

Joomla 3.0 Beta 1

@dongilbert
Copy link
Contributor

Mootools-more is not required for form validation to work in Joomla 2.5.x, so I'm not sure why they make that change, seeing that J!3 is more bent towards jQuery instead. I'll need to look into this more.

@piotrmocko
Copy link
Author

It is required to use function isValid()
in file: media/system/js/validate.js
which is a Joomla! from validator.
This function use "Hash" which is part of mootools-more.
It was correct in Joomla 2.5 but in Joomla 3.0 Beta 1 mootools-more was removed from validator

@piotrmocko
Copy link
Author

Please apply this update before releasing Joomla! 3.0 because my extension use this function from Joomla! validation script and it would not work

@dongilbert
Copy link
Contributor

Is this an extension you have distributed? Or one you are working on for a client? If it's the latter, you can add the call to JHtml::framework(true); to your code, and will bring in the required js. If it's a distributed extension, I think you could get away with pushing another update that does the same thing I mentioned above, and call it a 3.0 compatibility update. If you're users are upgrading their site to 3.0, then I don't think they'll have an issue with upgrading an extension as well. Also, they are probably comfortable with code, and could be just as easily instructed to add the same code block to the template's index.php file.

I'm only giving these suggestions because I'm not sure if this can make it in on time, since 3.0 is scheduled for release tomorrow.

@mbabker
Copy link
Contributor

mbabker commented Sep 26, 2012

Did Hash move between MooTools Core and More by chance?

@dongilbert
Copy link
Contributor

Looks like Hash was always in More, but in J!2.5.x, there was a MooTools 1.2 compatibility in mt-core that provided some type of hash compatibility. See https://github.com/joomla/joomla-cms/blob/2.5.x/media/system/js/mootools-core-uncompressed.js#L412 - That compat section been removed in the 3.0 branch.

So, to answer the question, it hasn't moved, but the compatibility layer was removed.

@mbabker
Copy link
Contributor

mbabker commented Sep 26, 2012

That removal was intended. So if we need to ensure MT More is loaded for that, then this fix will work. Care to do the pull request?

@piotrmocko
Copy link
Author

I have better solution. I have upgraded Joomla! validation script to work without Hash.
I have wrote part of code which do the same as Hash - converts object to array.

...

isValid: function(form)
{
    var valid = true;

    // Validate form fields
    var elements = form.getElements('fieldset').concat(Array.from(form.elements));
    for (var i=0;i < elements.length; i++) {
        if (this.validate(elements[i]) == false) {
            valid = false;
        }
    }

    // Run custom form validators if present
    var customHandlers = [];
    for (var key in this.custom) {
        if (this.custom.hasOwnProperty(key)) {
          customHandlers.push(this.custom[key]);  
        }
    }
    customHandlers.each(function(validator){
        if (validator.exec() != true) {
            valid = false;
        }
    });

    return valid;
}

@dongilbert
Copy link
Contributor

Pull request away - I like the that implementation since the validation script only requires that one simple feature of the Hash js. No need to load the full More library.

@piotrmocko
Copy link
Author

By the way I have related improvement into validation script posted on
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_id=8103&tracker_item_id=26114
Submitted By: Piotr Moćko
Adddate: 2012-02-28 11:46:24

@piotrmocko
Copy link
Author

In Joomla 3.0.1 it was not updated. Should I do something with this topic?

@pasamio
Copy link
Contributor

pasamio commented Oct 10, 2012

3.0.1 was primarily security and critical bug fixes around update work that needed to be done. It might get picked up in a later release if someone merges platform back down to the CMS after it gets fixed in the platform or alternatively you can push to get it fixed in the CMS. TBQH it sounds more like a CMS issue more than a platform issue.

@piotr-cz
Copy link
Contributor

Since MooTools 1.3 most of Hash functions has been moved to Object (which is part of Mootools Core) and documentation discourages using Hash class (When possible, please use the Object type!).

A while back I refactored the Validator script to act as a wrapper to MooTools native Form.Validator and optionally use HTML5 validations.

The code is heavily commented, contains backward compatibility methods to current version of validator.js and is simpler.

@piotrmocko
Copy link
Author

This code looks nice. I will have to test it. I think that it is high time to rebulid Joomla validation.

Check also if it would validate multiple select list as required

@piotr-cz
Copy link
Contributor

This hasn't been tested much, just few forms on frontend and backend in joomla 2.5.
The question howewer is JS framework. Code is tightly coupled to Mootools.

Description what I wanted to achieve

@alonextou
Copy link

Why was this ignored for 3.0.1, pasamio's reason is not good, because this is a simple and mandatory feature in Joomla's core. My registration form's return no value's. Is there a fix or branch for this I could use?

@dongilbert
Copy link
Contributor

You can edit the files for your site, and then when this get's merged into
core, you can drop your changes.

I wrote this up for overriding core Joomla classes with your own code -
https://gist.github.com/3237387

@alonextou
Copy link

Thanks dongilbert, the problem is I'm not too sure what I need to override, or what code I need to make it work. I was hoping someone who fixed this had a branch, or some code to copy. I'll give it a shot though.

@piotrmocko
Copy link
Author

Add to your form temporary PHP code:

self::framework(true);

to solve this issue.

You might also override JavaScript class with isValid method by adding following code to your form JS file:

JFormValidator.implement({
isValid: function(form)
{
    var valid = true;

    // Validate form fields
    var elements = form.getElements('fieldset').concat(Array.from(form.elements));
    for (var i=0;i < elements.length; i++) {
        if (this.validate(elements[i]) == false) {
            valid = false;
        }
    }

    // Run custom form validators if present
    var customHandlers = [];
    for (var key in this.custom) {
        if (this.custom.hasOwnProperty(key)) {
          customHandlers.push(this.custom[key]);  
        }
    }
    customHandlers.each(function(validator){
        if (validator.exec() != true) {
            valid = false;
        }
    });

    return valid;
}
});

@alonextou
Copy link

Thanks piotrmocko, but I changed self::framework(); to self::framework(true); in libraries/joomla/html/behavior.php, method formvalidation(), but it made no difference.

I am only trying to fix the Joomla registration form here, which does not return any warnings or errors.

@piotr-cz
Copy link
Contributor

@awc737:
I've tried registration on Joomla demo and looks like validation passed fine, fields receive classname invalid but there's no css styling attached to it so user has no easy change to pick up what's wrong. Is this what you are after?

If no, maybe we should move the discussion to Google groups..

pasamino is right, there's kind of confusion with javascript supplied with the platform. If platform is to be as slim as possible, these assets should move to the CMS. Besides Joomla 3.0.x is running on Platform 12.2, but current platform version is tagged 12.3.

@alonextou
Copy link

On that demo piotr-cz, shouldn't clicking 'Register' with all fields blank, or all invalid fields, at least return some kind of message, like a tool tip response or something?

@pasamio
Copy link
Contributor

pasamio commented Oct 18, 2012

If you click on the field it is highlighted red. Near as I can the field is tagged with the class "invalid" as is the label. The label also has an aria-linvalid true attribute as well. It would appear something is validating the field.

It is up to the template to decide how it wishes to render invalid fields. In this case the red highlighting doesn't occur unless you've clicked on the field. This is likely a bug in the CSS for the template that needs to be looked at so that it continues to highlight invalid fields that are not active. The template isn't something that the platform distributes but is in this case the CMS.

So as I noted before, I do not feel this is not a platform bug but a CMS bug. You're welcome to submit a pull request here to change that but we'll likely defer to the CMS folk as they're the ones most likely to be impacted by the change. However as I noted it doesn't mean it will immediately be available until the CMS chooses to pick it up from the Platform project.

Finally the Platform doesn't decide what goes in the CMS releases. The CMS is a downstream consumer of the Platform and as such we don't directly control what does or doesn't go into it's releases. While the CMS is certainly a source of fixes and improvements for the Platform, when it updates it's copy is on it's terms not ours. As I noted earlier the 3.0.1 release was a security release which the Joomla CMS typically stays away from making big changes in so that people aren't afraid that their site will be broken. This means only critical fixes and the security fixes usually go in. This is why something like this, apart from not even being fixed in the platform or a registered bug for the CMS, wasn't included or would be likely to be included.

@piotr-cz
Copy link
Contributor

@awc737 In my opinion it should immediately change border-color of invalid fields to red, like it has been happening in previous Joomla versions (maybe that's something that has been overlooked?).

Anyway as pasamino suggests, this can't be fixed here as this is a template thing and platform is not being shipped with templates. Try to add an issue or pull request in the CMS tracker, I'm pretty sure that more people are annoyed with this behavior.

Quick fix could be to prepend input.invalid, textarea.invalid, select.invalid, in template.css or if such behaviour works in the admin, replicate it for protostar template. Or check if/ how it's done for beez template.

We are discussing something quite different here: in some cases validator dependecies are not loaded/ how to make it lightweight.

Edit: This commit probably fixed invalid input filelds styling

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants