Page MenuHomePhabricator

Redirecting css/js wiki pages shouldn't cause syntax errors when loaded as a module.
Closed, ResolvedPublic

Description

Currentlym if SomeUser moves its [[User:SomeUser/vector.js]] to [[User:SomeUser/common.js]], the vector page will receive the redirect code such as

#REDIRECT [[User:SomeUser/common.js]]

which breaks the user scripts since this is not part of JS syntax (see a real case in the provided URL).

I think it is good idea to avoid this breakage somehow, e.g.:

  • Do not redirect the JS and CSS pages when moved
  • Add the redirect code /* inside of a comment */ (and make it work there, if it currently doesn't)
  • Add some kind of "importScript" or "mw.loader.load" function instead of a redirect, so that if a user is importing the old page it will still get the content of the script
  • Some other option...

Version: unspecified
Severity: normal
URL: https://secure.wikimedia.org/wikipedia/en/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=441581622#JavaScript_Problems
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=31827
https://bugzilla.wikimedia.org/show_bug.cgi?id=34930

Details

Reference
bz30074
TitleReferenceAuthorSource BranchDest Branch
Provide Digital Ocean Spaces credentials to buildkitdrepos/releng/gitlab-cloud-runner!12dduvallreview/buildkitd-cache-via-do-spacesmain
helm: Support providing S3 credentials via a secretrepos/releng/buildkit!7dduvallreview/support-s3-credentialswmf/v0.10
Add caching abilityrepos/releng/gitlab-cloud-runner!5jhuneidiT320746main
Draft: First attempt at caching for image buildingrepos/releng/blubber!8jhuneidiT320746main
Customize query in GitLab

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:52 PM
bzimport set Reference to bz30074.

john wrote:

Proposed fix

attachment b30074.patch ignored as obsolete

john wrote:

Follow up patch

Oops, this one trims the newline off the redirect

Attached:

Commenting it out seems like a weird way of avoiding this bug, besides it leaves an invalid redirect.

Either:

  • Don't allow moving without 'suppress redirect' option of css/js subpages
  • Change the loader to follow redirects
  • Change the loader to ignore redirects

john wrote:

(In reply to comment #3)

Commenting it out seems like a weird way of avoiding this bug, besides it
leaves an invalid redirect.

Either:

  • Don't allow moving without 'suppress redirect' option of css/js subpages

That no good for people who don't have the suppress redirect right, which are the people who are requesting this.

  • Change the loader to follow redirects

That sounds like a bad idea that could cause more trouble than it's worth.

  • Change the loader to ignore redirects

That might be doable, let me try it out.

john wrote:

Alternate method, redirect ignored in resourceloader

Attached:

Assigning to Roan for review.

Fixed in r94155. Thanks for the patch.

  • Bug 27509 has been marked as a duplicate of this bug. ***