jenkins-bot has submitted this change and it was merged. Change subject: Don't use parsed wikitext when dealing with CSS/JS ......................................................................
Don't use parsed wikitext when dealing with CSS/JS They're bogus and don't work and lead to fun unpredictable results when things like unclosed <h3> tags are used in comments. Ideally we could skip the parse entirely. Bug: 61752 Change-Id: Ife47af3c5a769d5b4697105527f93a18425d8e58 --- M includes/BuildDocument/PageDataBuilder.php M tests/browser/features/full_text.feature M tests/browser/features/step_definitions/search_steps.rb A tests/browser/features/support/articles/some.css A tests/browser/features/support/articles/some.js M tests/browser/features/support/hooks.rb 6 files changed, 57 insertions(+), 2 deletions(-) Approvals: Chad: Looks good to me, approved jenkins-bot: Verified Objections: Hashar: There's a problem with this change, please improve diff --git a/includes/BuildDocument/PageDataBuilder.php b/includes/BuildDocument/PageDataBuilder.php index 38ba644..105e830 100644 --- a/includes/BuildDocument/PageDataBuilder.php +++ b/includes/BuildDocument/PageDataBuilder.php @@ -35,8 +35,21 @@ ); public function build() { - foreach( $this->parseFuncs as $f ) { - $this->$f(); + switch ( $this->content->getModel() ) { + case CONTENT_MODEL_CSS: + case CONTENT_MODEL_JAVASCRIPT: + // Don't use parser output here. It's useless and leads + // to weird results. Instead, clear everything. See bug 61752. + $this->doc->add( 'category', array() ); + $this->doc->add( 'outgoing_link', array() ); + $this->doc->add( 'template', array() ); + $this->doc->add( 'file_text', array() ); + $this->doc->add( 'heading', array() ); + break; + default: + foreach( $this->parseFuncs as $f ) { + $this->$f(); + } } return $this->doc; diff --git a/tests/browser/features/full_text.feature b/tests/browser/features/full_text.feature index 8062bf1..3419886 100644 --- a/tests/browser/features/full_text.feature +++ b/tests/browser/features/full_text.feature @@ -191,3 +191,13 @@ Scenario: I can find things that Elaticsearch typically thinks of as word breaks in redirect title When I search for ยข Then Cent (currency) is the first search result + + @js_and_css + Scenario: JS pages don't corrupt the output + When I search for User:Tools/some.js jQuery + Then there is not alttitle on the first search result + + @js_and_css + Scenario: CSS pages don't corrupt the output + When I search for User:Tools/some.css jQuery + Then there is not alttitle on the first search result diff --git a/tests/browser/features/step_definitions/search_steps.rb b/tests/browser/features/step_definitions/search_steps.rb index b121259..d13833c 100644 --- a/tests/browser/features/step_definitions/search_steps.rb +++ b/tests/browser/features/step_definitions/search_steps.rb @@ -141,6 +141,9 @@ on(SearchResultsPage).first_result_highlighted_alttitle.should == highlighted end end +Then(/^there is not alttitle on the first search result$/) do + on(SearchResultsPage).first_result_alttitle_wrapper_element.should_not exist +end Then(/^(.+) is( not)? in the search results$/) do |title, not_searching| found = false on(SearchResultsPage).results.each do |result| diff --git a/tests/browser/features/support/articles/some.css b/tests/browser/features/support/articles/some.css new file mode 100644 index 0000000..9839344 --- /dev/null +++ b/tests/browser/features/support/articles/some.css @@ -0,0 +1,9 @@ +foo:before { + content: "<h3>"; + color: black; + background: #BDFFBD; +} + +foo:after jQuery { + content: "</h3>"; +} diff --git a/tests/browser/features/support/articles/some.js b/tests/browser/features/support/articles/some.js new file mode 100644 index 0000000..7bd73b8 --- /dev/null +++ b/tests/browser/features/support/articles/some.js @@ -0,0 +1,10 @@ +/*jshint browser: true, camelcase: true, curly: true, eqeqeq: true, immed: true, latedef: true, newcap: true, noarg: true, noempty: true, nonew: true, quotmark: true, undef: true, unused: true, strict: true, laxbreak: true, trailing: true, maxlen: 120, evil: true, onevar: true */ +/*global jQuery, console */ +(function($) { + 'use strict'; + var a = $('<h3>Correlatos<\/h3>'); + if (a === a) { + console.log(a); + } + // Note that even though this js isn't used for anything other than testing jslint will probably still check it. +})(jQuery); diff --git a/tests/browser/features/support/hooks.rb b/tests/browser/features/support/hooks.rb index 7319cd2..5e358c6 100644 --- a/tests/browser/features/support/hooks.rb +++ b/tests/browser/features/support/hooks.rb @@ -380,3 +380,13 @@ end $fallback_finder = true end + +Before("@js_and_css") do + if !$js_and_css + steps %Q{ + Given a page named User:Tools/Some.js exists with contents @some.js + And a page named User:Tools/Some.css exists with contents @some.css + } + end + $js_and_css = true +end -- To view, visit https://gerrit.wikimedia.org/r/115214 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ife47af3c5a769d5b4697105527f93a18425d8e58 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/CirrusSearch Gerrit-Branch: master Gerrit-Owner: Chad <ch...@wikimedia.org> Gerrit-Reviewer: Chad <ch...@wikimedia.org> Gerrit-Reviewer: Hashar <has...@free.fr> Gerrit-Reviewer: Manybubbles <never...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits