Skip to content

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

Merged
wilsonge merged 2 commits intostagingfrom
unknown repository
Jul 28, 2016
Merged

Fix#11155 Two finder module make invalid HTML for W3C#11168
wilsonge merged 2 commits intostagingfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jul 18, 2016 •
edited by brianteeman
Loading

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...

@ghost ghost mentioned this pull request Jul 18, 2016
Closed
@ggppdk
Copy link
Copy Markdown
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
Copy Markdown
Author

ghost commented Jul 18, 2016 •
edited by ghost
Loading

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

@brianteeman
Copy link
Copy Markdown
Contributor

brianteeman commented Jul 21, 2016

Doesnt the same change have to be made to mod_search?

dgrammatiko
dgrammatiko reviewed Jul 21, 2016
View reviewed changes
modules/mod_finder/tmpl/default.php
@@ -20,13 +20,13 @@
$lang->load('com_finder', JPATH_SITE);
Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko Jul 21, 2016

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
Copy Markdown
Author

@ghost ghost Jul 21, 2016

Choose a reason for hiding this comment

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

No. Without selector the default one is .advancedSelect.
https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/html/formbehavior.php#L40

@ghost
Copy link
Copy Markdown
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 ggppdk mentioned this pull request Jul 21, 2016
Merged
@ggppdk
Copy link
Copy Markdown
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
Copy Markdown

truptikagathara commented Jul 28, 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.

@brianteeman
Copy link
Copy Markdown
Contributor

brianteeman commented Jul 28, 2016

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 3.6.1

Development

Successfully merging this pull request may close these issues.

7 participants

@ggppdk @brianteeman @truptikagathara @dgrammatiko @wilsonge @joomla-cms-bot @bertmert