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

Fix#11155 Two finder module make invalid HTML for W3C #11168

Merged
merged 2 commits into from Jul 28, 2016
Merged

Fix#11155 Two finder module make invalid HTML for W3C #11168

merged 2 commits into from Jul 28, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jul 18, 2016

Pull Request for Issue #11155 .

Summary of Changes

Added postfix for attributes id and for in HTML and JS.

Testing Instructions

See #11155 please.

Test also with setting autosuggest if there are any conflicts with jquery.autocomplete.min.js

Test also with activated cache in module. I'm not sure...

@ggppdk
Copy link
Contributor

ggppdk commented Jul 18, 2016

Nice work but

why use a random number ?
in other cases we use module->id to make the HTML tag IDs unique,

  • using a random number makes impossible to locate the module by id via CSS or JS

@ghost
Copy link
Author

ghost commented Jul 18, 2016

@ggppdk
Yes, you're right. I was a bit tired ;-)

@brianteeman
Copy link
Contributor

Doesnt the same change have to be made to mod_search?

@@ -20,13 +20,13 @@
$lang->load('com_finder', JPATH_SITE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 15 also MUST change to JHtml::_('formbehavior.chosen', '.advancedSelect'); because if you call chosen without any selector the it will be applied to all select elements in the page and thus will break any functionality for 3PD extension that have javascript that manipulates any selects.

Copy link
Author

Choose a reason for hiding this comment

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

@ghost
Copy link
Author

ghost commented Jul 21, 2016

@brianteeman

Doesnt the same change have to be made to mod_search?

See new PR #11229
But we have a conflict in template Beez3 then.

@ggppdk
Copy link
Contributor

ggppdk commented Jul 21, 2016

I have tested this item ✅ successfully on 65e5538


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11168.

1 similar comment
@truptikagathara
Copy link

I have tested this item ✅ successfully on 65e5538


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11168.

@brianteeman
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11168.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 28, 2016
@brianteeman brianteeman added this to the Joomla 3.6.1 milestone Jul 28, 2016
@wilsonge wilsonge merged commit ac9ce1b into joomla:staging Jul 28, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants