jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/397860 )
Change subject: Update: change revision path param to oldid query param ...................................................................... Update: change revision path param to oldid query param Change the URL path parameter for wiki page revisions to a query parameter. e.g.: http://localhost:3000/wiki/Film/803321621#Notes ↓ http://localhost:3000/wiki/Film?oldid=803321621#Notes This change means that pages with previously ambiguous titles are now well-defined. e.g., 0/0 is known to be about indeterminate form and not the number zero at revision zero. Additionally, all slashes, even trailing, are now considered part of the title. This change was deemed necessary for API compatibility with existing links. Outdated references in the code were updated with: find src -type f| xargs -rd\\n sed -ri 's%(localhost.*)/([0-9]+)%\1?oldid=\2%g' Bug: T178616 Change-Id: If9220083f64c35afba105a01858167643076014d --- M package.json M src/common/components/content-page/content-page.css M src/common/components/content/styles/content.css M src/common/components/content/styles/core/gallery.css M src/common/components/content/styles/extensions/cite.css M src/common/components/content/styles/images.css M src/common/components/content/styles/lists.css M src/common/components/content/styles/tables.css M src/common/components/content/styles/templates/plainlist.css M src/common/pages/home.tsx M src/common/pages/wiki.tsx M src/common/router/routes.test.ts M src/common/router/routes.ts 13 files changed, 98 insertions(+), 106 deletions(-) Approvals: Jhernandez: Looks good to me, approved jenkins-bot: Verified diff --git a/package.json b/package.json index bd3f051..f925207 100644 --- a/package.json +++ b/package.json @@ -130,7 +130,7 @@ }, { "path": "dist/public/pages/wiki.*.js", - "maxSize": "2.8KB" + "maxSize": "2.9KB" } ] } diff --git a/src/common/components/content-page/content-page.css b/src/common/components/content-page/content-page.css index f9a3d4f..83dc3ca 100644 --- a/src/common/components/content-page/content-page.css +++ b/src/common/components/content-page/content-page.css @@ -2,7 +2,7 @@ /* The cContent should never allow content overlap. Add an empty node that clears the left and right so that the content container can wrap its contents fully even if they're floated. - http://localhost:3000/wiki/CSNK2A2/774042184 */ + http://localhost:3000/wiki/CSNK2A2?oldid=774042184 */ content: ""; clear: both; diff --git a/src/common/components/content/styles/content.css b/src/common/components/content/styles/content.css index 1446043..dbf50c1 100644 --- a/src/common/components/content/styles/content.css +++ b/src/common/components/content/styles/content.css @@ -9,8 +9,8 @@ /* On very small screens, lengthy words such as references often overflow which causes the whole page to scroll horizontally. When there's no other alternative, break the word down at the character level. - http://localhost:3000/wiki/Film/803321621#Notes - http://localhost:3000/wiki/China/804850001#Telecommunications */ + http://localhost:3000/wiki/Film?oldid=803321621#Notes + http://localhost:3000/wiki/China?oldid=804850001#Telecommunications */ overflow-wrap: break-word; } @@ -56,7 +56,7 @@ .Content .quotebox { /* Allow quoteboxes with hardcoded widths to stay within the window boundaries. - http://localhost:3000/wiki/The_Beatles/804487790#Sgt._Pepper.27s_Lonely_Hearts_Club_Band */ + http://localhost:3000/wiki/The_Beatles?oldid=804487790#Sgt._Pepper.27s_Lonely_Hearts_Club_Band */ width: auto !important; } @@ -92,7 +92,7 @@ /* Preformatted text frequently overflows horizontally. Provide a horizontal scrollbar when needed since wrapping monospaced content is often illegible. - http://localhost:3000/wiki/TypeScript/801562762#Classes */ + http://localhost:3000/wiki/TypeScript?oldid=801562762#Classes */ .Content pre { overflow-x: auto; } @@ -100,8 +100,8 @@ /* Accessibility */ /* Hide (formatting) elements from screen, but not from screenreaders. - http://localhost:3000/wiki/MediaWiki:Common.css/804730289 - http://localhost:3000/wiki/1_+_1_+_1_+_1_+_⋯/793725799 */ + http://localhost:3000/wiki/MediaWiki:Common.css?oldid=804730289 + http://localhost:3000/wiki/1_+_1_+_1_+_1_+_⋯?oldid=793725799 */ .Content .visualhide { position: absolute; left: -10000px; diff --git a/src/common/components/content/styles/core/gallery.css b/src/common/components/content/styles/core/gallery.css index 3c4b120..72291bd 100644 --- a/src/common/components/content/styles/core/gallery.css +++ b/src/common/components/content/styles/core/gallery.css @@ -1,6 +1,6 @@ /* Galleries https://phabricator.wikimedia.org/source/mediawiki/browse/master/resources/src/mediawiki/page/gallery.css;43e3ff8dcd1eb161ef896602ffcbb8134b44c662 - http://localhost:3000/wiki/Pablo_Picasso/804068218#Synthetic_cubism:_1912.E2.80.931919 */ + http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218#Synthetic_cubism:_1912.E2.80.931919 */ /* These display attributes look nonsensical, but are needed to support IE and FF2 */ /* Don't forget to update gallery.print.css */ diff --git a/src/common/components/content/styles/extensions/cite.css b/src/common/components/content/styles/extensions/cite.css index ca0b7ff..3827f9d 100644 --- a/src/common/components/content/styles/extensions/cite.css +++ b/src/common/components/content/styles/extensions/cite.css @@ -1,6 +1,6 @@ /* CSS for Parsoid Cite extension https://phabricator.wikimedia.org/diffusion/ECIT/browse/master/modules/ext.cite.style.css;b58db89aac6202f96c34009e878a33196827c72b - http://localhost:3000/wiki/Pablo_Picasso/804068218 */ + http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218 */ /* Style for <ref>s */ span.mw-ref { diff --git a/src/common/components/content/styles/images.css b/src/common/components/content/styles/images.css index 50c4b78..e8d6fef 100644 --- a/src/common/components/content/styles/images.css +++ b/src/common/components/content/styles/images.css @@ -1,17 +1,17 @@ /* Containers */ -/* http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)/809904389#Singles */ +/* http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)?oldid=809904389#Singles */ .Content figure { margin: 0; } -/* http://localhost:3000/wiki/China/804850001#Landscape_and_climate */ +/* http://localhost:3000/wiki/China?oldid=804850001#Landscape_and_climate */ .Content .thumbinner { width: auto !important; height: auto !important; } -/* http://localhost:3000/wiki/China/804850001#Landscape_and_climate */ +/* http://localhost:3000/wiki/China?oldid=804850001#Landscape_and_climate */ .Content .thumbinner > div { /* Center text and images within the div. */ text-align: center; @@ -20,7 +20,7 @@ /* Images */ .Content img, -/* http://localhost:3000/wiki/Film/803321621#Early_evolution */ +/* http://localhost:3000/wiki/Film?oldid=803321621#Early_evolution */ .Content video { /* Limit videos and images to the window width and always proportionally scale height. */ @@ -31,19 +31,19 @@ vertical-align: middle; } -/* http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)/809904389#Singles - http://localhost:3000/wiki/The_Beatles/810732238#Beatles_for_Sale.2C_Help.21_and_Rubber_Soul */ +/* http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)?oldid=809904389#Singles + http://localhost:3000/wiki/The_Beatles?oldid=810732238#Beatles_for_Sale.2C_Help.21_and_Rubber_Soul */ .Content .mbox-small-left video, .Content .mbox-small video { height: 32px !important; } /* Captions */ -/* http://localhost:3000/wiki/Pablo_Picasso/804068218#Early_life */ +/* http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218#Early_life */ .Content figcaption, -/* http://localhost:3000/wiki/Pablo_Picasso/804068218#Before_1900 */ +/* http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218#Before_1900 */ .Content .thumbcaption, -/* http://localhost:3000/wiki/Pablo_Picasso/804068218#Synthetic_cubism:_1912.E2.80.931919 */ +/* http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218#Synthetic_cubism:_1912.E2.80.931919 */ .Content .gallerytext { margin: .5em 0; font-size: var(--font-size-caption); diff --git a/src/common/components/content/styles/lists.css b/src/common/components/content/styles/lists.css index 9361f32..b8de9de 100644 --- a/src/common/components/content/styles/lists.css +++ b/src/common/components/content/styles/lists.css @@ -2,14 +2,14 @@ .Content .hlist dt, .Content .hlist li { /* Display items inline instead of one after another. - http://localhost:3000/wiki/The_Beatles/804487790 */ + http://localhost:3000/wiki/The_Beatles?oldid=804487790 */ display: inline; } .Content .hlist dd:after, .Content .hlist li:after { /* Add a bullet to every item. - http://localhost:3000/wiki/The_Beatles/804487790 */ + http://localhost:3000/wiki/The_Beatles?oldid=804487790 */ content: " · "; font-weight: 700; } diff --git a/src/common/components/content/styles/tables.css b/src/common/components/content/styles/tables.css index 252ea89..d29300c 100644 --- a/src/common/components/content/styles/tables.css +++ b/src/common/components/content/styles/tables.css @@ -1,19 +1,19 @@ /* Tables and infobox */ .Content > table, -/* http://localhost:3000/wiki/Plasma_(physics)/804781490#Research */ +/* http://localhost:3000/wiki/Plasma_(physics)?oldid=804781490#Research */ .Content table.multicol { /* Table cells, including wikitext tables and infoboxes, support arbitrary widths. As a well used feature, small screens are unfortunately often not considered so universally change the presentation to a block-element and provide a horizontal scrollbar when needed. - http://localhost:3000/wiki/Periodic_table/803535152#Overview + http://localhost:3000/wiki/Periodic_table?oldid=803535152#Overview http://localhost:3000/wiki/Help:Table#Width.2C_height */ overflow-x: auto; display: block; /* Tables themselves support widths too and must be limited. - http://localhost:3000/wiki/Periodic_table_(large_cells)/799715731 */ + http://localhost:3000/wiki/Periodic_table_(large_cells)?oldid=799715731 */ max-width: 100%; /* Center all tables. */ @@ -32,7 +32,7 @@ /* Keep cells wide enough to contain their contents. This resets the more general overflow-wrap rule applied to all page text in content.css. - http://localhost:3000/wiki/Periodic_table_(large_cells)/799715731#32-column */ + http://localhost:3000/wiki/Periodic_table_(large_cells)?oldid=799715731#32-column */ overflow-wrap: normal; } @@ -40,14 +40,14 @@ background-color: var(--wmui-color-base90); border: 1px solid var(--wmui-color-base80); - /* http://localhost:3000/wiki/Wolverine/804085375 */ + /* http://localhost:3000/wiki/Wolverine?oldid=804085375 */ width: auto !important; max-width: 22em !important; } @media (min-width: 1000px) { .Content .infobox { - /* http://localhost:3000/wiki/Wolverine/804085375 */ + /* http://localhost:3000/wiki/Wolverine?oldid=804085375 */ float: right; margin: 0 0 1em 1em; } @@ -61,38 +61,38 @@ .Content .infobox td, .Content .infobox th { - /* http://localhost:3000/wiki/China/804850001 (under "Transcriptions") */ + /* http://localhost:3000/wiki/China?oldid=804850001 (under "Transcriptions") */ text-align: left; border-top: 1px solid var(--wmui-color-base80); } .Content .infobox tr:first-child > td, .Content .infobox tr:first-child > th { - /* http://localhost:3000/wiki/China/804850001 ("Transcriptions") - http://localhost:3000/wiki/Pablo_Picasso/804068218 ("Pablo Picasso") */ + /* http://localhost:3000/wiki/China?oldid=804850001 ("Transcriptions") + http://localhost:3000/wiki/Pablo_Picasso?oldid=804068218 ("Pablo Picasso") */ border-top-width: 0; } -/* http://localhost:3000/wiki/Periodic_table_(large_cells)/799715731#Notes */ +/* http://localhost:3000/wiki/Periodic_table_(large_cells)?oldid=799715731#Notes */ .Content .navbox, -/* http://localhost:3000/wiki/Periodic_table/803535152#External_links */ +/* http://localhost:3000/wiki/Periodic_table?oldid=803535152#External_links */ .Content .metadata { overflow-x: auto; } -/* http://localhost:3000/wiki/Voodoo_Shoppe/443846926 */ +/* http://localhost:3000/wiki/Voodoo_Shoppe?oldid=443846926 */ .Content .navbox .navbox-inner { width: 100%; } -/* http://localhost:3000/wiki/MediaWiki:Common.css/806220319 - http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)/809904389#Singles */ +/* http://localhost:3000/wiki/MediaWiki:Common.css?oldid=806220319 + http://localhost:3000/wiki/Girl_(Pharrell_Williams_album)?oldid=809904389#Singles */ .Content .mbox-small-left { margin: var(--half-space) var(--space) 0 var(--space); width: 400px; } -/* http://localhost:3000/wiki/The_Beatles/810732238#Beatles_for_Sale.2C_Help.21_and_Rubber_Soul */ +/* http://localhost:3000/wiki/The_Beatles?oldid=810732238#Beatles_for_Sale.2C_Help.21_and_Rubber_Soul */ .Content .mbox-small { margin: var(--half-space) 0; } diff --git a/src/common/components/content/styles/templates/plainlist.css b/src/common/components/content/styles/templates/plainlist.css index e157163..ae78c42 100644 --- a/src/common/components/content/styles/templates/plainlist.css +++ b/src/common/components/content/styles/templates/plainlist.css @@ -1,5 +1,5 @@ -/* http://localhost:3000/wiki/Template:Plainlist/628943872 - http://localhost:3000/wiki/China/804850001 (infobox) */ +/* http://localhost:3000/wiki/Template:Plainlist?oldid=628943872 + http://localhost:3000/wiki/China?oldid=804850001 (infobox) */ .Content .plainlist > ul { line-height: inherit; diff --git a/src/common/pages/home.tsx b/src/common/pages/home.tsx index 2eea50a..23249fa 100644 --- a/src/common/pages/home.tsx +++ b/src/common/pages/home.tsx @@ -35,15 +35,14 @@ { title: "Cheese_cake", text: "Redirect page (302)" }, { title: "Ice_cream_cake", - revision: "24242119", - text: "An arbitrary revision" + revision: 24242119, + text: "An arbitrary revision and query string" }, { title: "File:Vanilla_Ice_Cream_Cone_at_Camp_Manitoulin.jpg", text: "Redirect (external) and File page" }, - { title: "Nonexistent_title", text: "Missing (404)" }, - { title: "Pizza", query: { foo: "bar" }, text: "Query string" } + { title: "Nonexistent_title", text: "Missing (404)" } ]; export default { @@ -78,20 +77,21 @@ {testPages.map( ({ title, - query, revision, text }: { title: PageTitleID | string; - query?: QueryParams; - revision?: string; + revision?: number; text: string; }) => ( <li> <Link href={wiki.toPathQuery({ - path: { title, revision }, - query + path: { title }, + query: + revision === undefined + ? undefined + : { oldid: `${revision}` } })} > {text} diff --git a/src/common/pages/wiki.tsx b/src/common/pages/wiki.tsx index 0a98317..ac457ae 100644 --- a/src/common/pages/wiki.tsx +++ b/src/common/pages/wiki.tsx @@ -22,13 +22,19 @@ * (encoded, not necessarily denormalized) PageTitlePath. */ title: PageTitleID | PageTitlePath | string; - revision?: string; + }; + query?: { + /** The page revision. */ + oldid?: string; }; } // undefined means random input (Route.toPath()) and {} means random output // (Route.toParams()). -export type Params = PageParams | { path?: undefined } | undefined; +export type Params = + | PageParams + | { path?: undefined; query?: undefined } + | undefined; export interface Props { page: PageModel; @@ -44,19 +50,16 @@ : { titlePath: params.path.title, revision: - params.path.revision === undefined + params.query === undefined || params.query.oldid === undefined ? undefined - : parseInt(params.path.revision, 10), + : parseInt(params.query.oldid, 10), init } ).catch(error => { if (error instanceof RedirectError) { const title = unmarshalPageTitleID(error.url); const url = wiki.toPath({ - path: { - title, - revision: params.path ? params.path.revision : undefined - } + path: { title, oldid: params.query ? params.query.oldid : undefined } }); throw new RedirectError(error.status, url); } diff --git a/src/common/router/routes.test.ts b/src/common/router/routes.test.ts index e1ec911..961b893 100644 --- a/src/common/router/routes.test.ts +++ b/src/common/router/routes.test.ts @@ -82,94 +82,97 @@ name: "wiki (title)", route: wiki, path: "/wiki/title", - params: { path: { title: "title", revision: undefined } } + params: { path: { title: "title" } } }, { name: "wiki (title, revision)", route: wiki, - path: "/wiki/title/1", - params: { path: { title: "title", revision: "1" } } + path: "/wiki/title", + query: "oldid=1", + params: { path: { title: "title" }, query: { oldid: "1" } } }, { name: "wiki (title with slash)", route: wiki, path: "/wiki/title/text", - params: { path: { title: "title/text", revision: undefined } } + params: { path: { title: "title/text" } } }, { name: "wiki (title with slash, revision)", route: wiki, - path: "/wiki/title/text/123456789", - params: { path: { title: "title/text", revision: "123456789" } } + path: "/wiki/title/text", + query: "oldid=123456789", + params: { path: { title: "title/text" }, query: { oldid: "123456789" } } }, { name: "wiki (title with trailing slash)", route: wiki, path: "/wiki/title/", - params: { path: { title: "title", revision: undefined } } + params: { path: { title: "title/" } } }, { name: "wiki (title with trailing slash, revision)", route: wiki, - path: "/wiki/title/123456789/", - params: { path: { title: "title", revision: "123456789" } } + path: "/wiki/title/", + query: "oldid=123456789", + params: { path: { title: "title/" }, query: { oldid: "123456789" } } }, { name: "wiki (title with slash and trailing slash)", route: wiki, path: "/wiki/title/text/", - params: { path: { title: "title/text", revision: undefined } } + params: { path: { title: "title/text/" } } }, { name: "wiki (title with slash and trailing slash, revision)", route: wiki, - path: "/wiki/title/text/123456789/", - params: { path: { title: "title/text", revision: "123456789" } } + path: "/wiki/title/text/", + query: "oldid=123456789", + params: { + path: { title: "title/text/" }, + query: { oldid: "123456789" } + } }, { name: "wiki (title is a slash)", route: wiki, path: "/wiki//", - params: { path: { title: "/", revision: undefined } } + params: { path: { title: "/" } } }, { name: "wiki (title is a slash, revision)", route: wiki, - path: "/wiki///123456789/", - params: { path: { title: "/", revision: "123456789" } } + path: "/wiki//", + query: "oldid=123456789", + params: { path: { title: "/" }, query: { oldid: "123456789" } } }, { name: "wiki (title is a question mark)", route: wiki, path: "/wiki/%3f", - params: { path: { title: "%3f", revision: undefined } } + params: { path: { title: "%3f" } } }, { name: "wiki (title is a question mark, revision)", route: wiki, - path: "/wiki/%3f/123456789/", - params: { path: { title: "%3f", revision: "123456789" } } + path: "/wiki/%3f", + query: "oldid=123456789", + params: { path: { title: "%3f" }, query: { oldid: "123456789" } } }, { name: "wiki (title with every supported character class)", route: wiki, path: "/wiki/ %!\"$&'()*,-./0:;=@A\\^_`a~\x80+", - params: { - path: { - title: " %!\"$&'()*,-./0:;=@A\\^_`a~\x80+", - revision: undefined - } - } + params: { path: { title: " %!\"$&'()*,-./0:;=@A\\^_`a~\x80+" } } }, { name: "wiki (title with every supported character class, revision)", route: wiki, - path: "/wiki/ %!\"$&'()*,-./0:;=@A\\^_`a~\x80+/123456789", + path: "/wiki/ %!\"$&'()*,-./0:;=@A\\^_`a~\x80+", + query: "oldid=123456789", params: { - path: { - title: " %!\"$&'()*,-./0:;=@A\\^_`a~\x80+", - revision: "123456789" - } + path: { title: " %!\"$&'()*,-./0:;=@A\\^_`a~\x80+" }, + query: { oldid: "123456789" } } }, { @@ -188,7 +191,7 @@ name: "summary (title with trailing slash)", route: summary, path: "/page/summary/title/", - params: { path: { title: "title" } } + params: { path: { title: "title/" } } }, { name: "summary (title is a slash)", @@ -200,7 +203,7 @@ name: "summary (title with slash and trailing slash)", route: summary, path: "/page/summary/title/text/", - params: { path: { title: "title/text" } } + params: { path: { title: "title/text/" } } }, { name: "summary (title is a question mark)", @@ -238,11 +241,5 @@ describe("wiki", () => { it("a title with an illegal character fails", () => assert.deepStrictEqual(wiki.toParams("/wiki/{title}"), undefined)); - - it("a nonnumerical revision fails", () => - assert.deepStrictEqual( - wiki.toParams("/wiki/title/{revision}"), - undefined - )); }); }); diff --git a/src/common/router/routes.ts b/src/common/router/routes.ts index 5ce6401..a83a470 100644 --- a/src/common/router/routes.ts +++ b/src/common/router/routes.ts @@ -2,30 +2,22 @@ import { Params as SummaryParams } from "../pages/summary"; import { NoParamsRoute, Route, newRoute } from "./route"; -export const home: NoParamsRoute = newRoute({ - path: "/", - page: "home" -}); +export const home: NoParamsRoute = newRoute({ path: "/", page: "home" }); -export const about: NoParamsRoute = newRoute({ - path: "/about", - page: "about" -}); +export const about: NoParamsRoute = newRoute({ path: "/about", page: "about" }); +// https://www.mediawiki.org/wiki/Manual:$wgLegalTitleChars +// https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo const TITLE_CHARACTER_REGEX_STRING = "[ %!\"$&'\\(\\)*,\\-.\\/0-9:;=@A-Z\\\\^_`a-z~\\x80-\\xFF+]"; export const wiki: Route<WikiParams> = newRoute({ - // https://www.mediawiki.org/wiki/Manual:$wgLegalTitleChars - // https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo - path: `/wiki/:title(${TITLE_CHARACTER_REGEX_STRING}+?)/:revision(\\d+)?`, // eslint-disable-line max-len + path: `/wiki/:title(${TITLE_CHARACTER_REGEX_STRING}+)`, page: "wiki" }); export const summary: Route<SummaryParams> = newRoute({ - // Note: title is specified as non-greedy here only to omit a trailing slash - // from the title as the wiki endpoint does. - path: `/page/summary/:title(${TITLE_CHARACTER_REGEX_STRING}+?)`, + path: `/page/summary/:title(${TITLE_CHARACTER_REGEX_STRING}+)`, page: "summary" }); -- To view, visit https://gerrit.wikimedia.org/r/397860 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: If9220083f64c35afba105a01858167643076014d Gerrit-PatchSet: 4 Gerrit-Project: marvin Gerrit-Branch: master Gerrit-Owner: Niedzielski <[email protected]> Gerrit-Reviewer: Jhernandez <[email protected]> Gerrit-Reviewer: Sniedzielski <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
