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
