From 7cfa7c22711bc0934bdcd4184f690e5f13d629fb Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Thu, 14 Nov 2024 10:22:47 -0800 Subject: [PATCH 1/6] Re-work image sizing algorithm. It now enforces a maximum image width of 320px, assuming it can use URL hacking to generate the proper URL to download --- src/renderers/wikimedia-mobile.renderer.ts | 55 ++++++++-- test/unit/renderers/mobile.renderer.test.ts | 108 ++++++++++++++------ 2 files changed, 127 insertions(+), 36 deletions(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index b3c64047..ba5fbe63 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -7,6 +7,9 @@ import { RenderOpts, RenderOutput } from './abstract.renderer.js' type PipeFunction = (value: DominoElement) => DominoElement | Promise +const THUMB_WIDTH_REGEX = /\/(\d+)px-[^/]+$/ +const THUMB_MAX_WIDTH = 320 + // Represent 'https://{wikimedia-wiki}/api/rest_v1/page/mobile-html/' export class WikimediaMobileRenderer extends MobileRenderer { constructor() { @@ -104,15 +107,55 @@ export class WikimediaMobileRenderer extends MobileRenderer { // Set the attributes for the img element based on the data attributes in the span // The data-data-file-original-src attribute is the URL of the image that was used in the original article. - // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up to 320px - // or 640px in order to be "full width" on mobile devices. However, if the mobile API didn't scale the - // image up, then the data-data-file-original-src attribute will be missing, and we should use the data-src. + // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up in order to + // be "full width" on mobile devices. However, if the mobile API didn't scale the image up, then the + // data-data-file-original-src attribute will be missing, and we should use the data-src. // See https://github.com/openzim/mwoffliner/issues/1925. - const imgSrc = span.getAttribute('data-data-file-original-src') || span.getAttribute('data-src') + let originalWidth: number + let match: RegExpMatchArray | undefined + const originalSrc = span.getAttribute('data-data-file-original-src') + if (originalSrc) { + // Try to match against an image URL with a width in it. + match = THUMB_WIDTH_REGEX.exec(originalSrc) + if (match) { + originalWidth = parseInt(match[1], 10) + } + } + + // These are the attributes that were "prepared" for us by the mobile-html endpoint. + const preparedSrc = span.getAttribute('data-src') + const preparedWidth = parseInt(span.getAttribute('data-width') || '0', 10) + const preparedHeight = parseInt(span.getAttribute('data-height') || '0', 10) + + let imgSrc = preparedSrc + let width = preparedWidth + let height: number + if (originalWidth && match && originalWidth < preparedWidth) { + // There was a match on the originalSrc, and it is an image that is smaller than the prepared image. + width = originalWidth + imgSrc = originalSrc + } + if (THUMB_MAX_WIDTH < originalWidth && THUMB_MAX_WIDTH < preparedWidth) { + let srcToReplace = originalSrc + if (!match) { + // Try to match against the prepared URL, it might have sizing information. + match = THUMB_WIDTH_REGEX.exec(preparedSrc) + srcToReplace = preparedSrc + } + // If there is no match, we will just use the prepared image as is. + if (match) { + width = THUMB_MAX_WIDTH + imgSrc = srcToReplace.replace(`${match[1]}px`, `${width}px`) + } + } + // If the above ifs didn't execute, we're using the prepared image. + // This is a no-op if width == preparedWidth. + height = Math.round((preparedHeight * width) / preparedWidth) + img.src = urlJoin(protocol, imgSrc) img.setAttribute('decoding', 'async') - img.width = span.getAttribute('data-width') - img.height = span.getAttribute('data-height') + img.width = width + img.height = height img.className = span.getAttribute('data-class') // Replace the span with the img element diff --git a/test/unit/renderers/mobile.renderer.test.ts b/test/unit/renderers/mobile.renderer.test.ts index 44f6487c..393bf1df 100644 --- a/test/unit/renderers/mobile.renderer.test.ts +++ b/test/unit/renderers/mobile.renderer.test.ts @@ -1,14 +1,14 @@ import * as domino from 'domino' import { WikimediaMobileRenderer } from '../../../src/renderers/wikimedia-mobile.renderer' +import exp from 'constants' describe('mobile renderer', () => { - let window - describe('unhiding sections', () => { + let test_window beforeEach(() => { // Snippet of an article with nested hidden sections. - window = domino.createWindow( + test_window = domino.createWindow( `
@@ -35,7 +35,7 @@ describe('mobile renderer', () => { test('it removes the hidden class from sections', async () => { const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.unhideSections(window.document) + const actual = mobileRenderer.INTERNAL.unhideSections(test_window.document) const sections = actual.querySelectorAll('section') expect(sections.length).toBe(2) @@ -45,8 +45,8 @@ describe('mobile renderer', () => { }) describe('image converter', () => { - beforeEach(() => { - window = domino.createWindow( + test('it converts lazy load to images with the proper sizes', async () => { + const test_window = domino.createWindow( `
@@ -94,22 +94,6 @@ describe('mobile renderer', () => {
  • Bamakɔ Kura
  • Jikoroni
  • Bakɔ jikɔrɔni (= derrière le fleuve)
    • -
  • Misira
    • -
  • Medina Kura
    • -
  • Bankɔni
    • -
  • Maɲambugu
    • -
  • Dravela
    • -
  • Jɛlibugu
    • -
  • Bolibana
    • -
  • Wɔlɔfɔbugu
    • -
  • Bajalan I,II,III
    • -
  • ɲarela
    • -
  • Bagadaji
    • -
  • Bozola
    • -
  • Falaje
    • -
  • ɲamakoro
    • -
  • Sɛbenikɔrɔ
    • -
  • Quinzanbugu
  • Kinsanbugu
  • Amdalayɛ
  • Sabalibugu
    • @@ -121,35 +105,68 @@ describe('mobile renderer', () => { `, 'http://bm.wikipedia.org/api/rest_v1/page/mobile-html/BamakBamakɔ', ) - }) - test('it converts lazy load to images with the proper size', async () => { const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(window.document) + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') const imgs = actual.querySelectorAll('img') expect(spans.length).toBe(0) expect(imgs.length).toBe(2) expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/8/8f/Bamako_et_fleuve_Niger.jpg/250px-Bamako_et_fleuve_Niger.jpg') + expect(imgs[0].width).toEqual(250) + expect(imgs[0].height).toEqual(188) expect(imgs[1].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/2/20/Bamako_bridge2.jpg/250px-Bamako_bridge2.jpg') + expect(imgs[1].width).toEqual(250) + expect(imgs[1].height).toEqual(167) + }) + + test('uses max width of 320 when src and data-data-file-original-src are both bigger', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') + expect(imgs[0].width).toEqual(320) + expect(imgs[0].height).toEqual(320) }) - test('it uses the data-src when data-data-file-original-src is not available', async () => { + test('uses original src width when it is the smallest', async () => { const test_window = domino.createWindow( ` `, 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', @@ -163,6 +180,37 @@ describe('mobile renderer', () => { expect(spans.length).toBe(0) expect(imgs.length).toBe(1) expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/150px-BMW.svg.png') + expect(imgs[0].width).toEqual(150) + expect(imgs[0].height).toEqual(150) + }) + test('uses prepared src when there is no original src, and no way to URL hack', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/BMW.svg.png') + expect(imgs[0].width).toEqual(800) + expect(imgs[0].height).toEqual(800) }) }) }) From 66c6c362275ae9bf87e5a2b3938f4eeb51a68a17 Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Thu, 14 Nov 2024 10:42:59 -0800 Subject: [PATCH 2/6] Fix lint --- src/renderers/wikimedia-mobile.renderer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index ba5fbe63..ad54c45d 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -129,7 +129,6 @@ export class WikimediaMobileRenderer extends MobileRenderer { let imgSrc = preparedSrc let width = preparedWidth - let height: number if (originalWidth && match && originalWidth < preparedWidth) { // There was a match on the originalSrc, and it is an image that is smaller than the prepared image. width = originalWidth @@ -150,7 +149,7 @@ export class WikimediaMobileRenderer extends MobileRenderer { } // If the above ifs didn't execute, we're using the prepared image. // This is a no-op if width == preparedWidth. - height = Math.round((preparedHeight * width) / preparedWidth) + const height = Math.round((preparedHeight * width) / preparedWidth) img.src = urlJoin(protocol, imgSrc) img.setAttribute('decoding', 'async') From 10ccaa5a819dda922b57429789b55bd7471bca5b Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Sun, 17 Nov 2024 14:24:36 -0800 Subject: [PATCH 3/6] Fix edge case where original src does not exist --- src/renderers/wikimedia-mobile.renderer.ts | 4 ++- test/unit/renderers/mobile.renderer.test.ts | 28 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index ad54c45d..52044722 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -134,7 +134,9 @@ export class WikimediaMobileRenderer extends MobileRenderer { width = originalWidth imgSrc = originalSrc } - if (THUMB_MAX_WIDTH < originalWidth && THUMB_MAX_WIDTH < preparedWidth) { + if (THUMB_MAX_WIDTH < originalWidth || (!originalWidth && THUMB_MAX_WIDTH < preparedWidth)) { + // If both srcs are too big, try to either use the original URL hacking, or URL hacking on the + // "prepared" src, to get an image of the right size. let srcToReplace = originalSrc if (!match) { // Try to match against the prepared URL, it might have sizing information. diff --git a/test/unit/renderers/mobile.renderer.test.ts b/test/unit/renderers/mobile.renderer.test.ts index 393bf1df..34b220e7 100644 --- a/test/unit/renderers/mobile.renderer.test.ts +++ b/test/unit/renderers/mobile.renderer.test.ts @@ -152,7 +152,35 @@ describe('mobile renderer', () => { expect(imgs[0].width).toEqual(320) expect(imgs[0].height).toEqual(320) }) + test('uses max width of 320 when there is no original src and the prepared src is too big', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') + expect(imgs[0].width).toEqual(320) + expect(imgs[0].height).toEqual(320) + }) test('uses original src width when it is the smallest', async () => { const test_window = domino.createWindow( ` From 666e60efb39212f89f09decbd8852540c4917794 Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Tue, 19 Nov 2024 12:38:23 -0800 Subject: [PATCH 4/6] Introduce helper data structure and function to simplify logic. Scale image in dominant dimension (width or height) --- src/renderers/wikimedia-mobile.renderer.ts | 149 ++++++---- test/unit/renderers/mobile.renderer.test.ts | 287 ++++++++++++++------ 2 files changed, 303 insertions(+), 133 deletions(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index 52044722..2fbf866e 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -8,7 +8,13 @@ import { RenderOpts, RenderOutput } from './abstract.renderer.js' type PipeFunction = (value: DominoElement) => DominoElement | Promise const THUMB_WIDTH_REGEX = /\/(\d+)px-[^/]+$/ -const THUMB_MAX_WIDTH = 320 +const THUMB_MAX_DIMENSION = 320 + +declare interface ImageMetadata { + src: string | null + width: number + height: number +} // Represent 'https://{wikimedia-wiki}/api/rest_v1/page/mobile-html/' export class WikimediaMobileRenderer extends MobileRenderer { @@ -96,67 +102,108 @@ export class WikimediaMobileRenderer extends MobileRenderer { return doc } + private calculateImageDimensions(span: DominoElement) { + // These are the attributes that were "prepared" for us by the mobile-html endpoint. + const preparedData = { + src: span.getAttribute('data-src'), + width: parseInt(span.getAttribute('data-width') || '0', 10), + height: parseInt(span.getAttribute('data-height') || '0', 10), + } + + // Calculate the ratio so we know if we're scaling down in the width or height dimension. + const widthHeightRatio = preparedData.width / preparedData.height + let scaleUsingHeight = widthHeightRatio < 1.0 + + // The data-data-file-original-src attribute is the URL of the image that was used in the original article. + // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up in order to + // be "full width" on mobile devices. However, if the mobile API didn't scale the image up, then the + // data-data-file-original-src attribute will be missing, and we should use the data-src. + // See https://github.com/openzim/mwoffliner/issues/1925. + let originalData: ImageMetadata | undefined + const originalSrc = span.getAttribute('data-data-file-original-src') + if (originalSrc) { + // Try to match against an image URL with a width in it. + const match = THUMB_WIDTH_REGEX.exec(originalSrc) + if (match) { + const originalWidth = parseInt(match[1], 10) + originalData = { + src: originalSrc, + width: originalWidth, + height: Math.round(originalWidth / widthHeightRatio), + } + } + } + + let maxData: ImageMetadata | undefined + if (scaleUsingHeight) { + maxData = { + src: null, + width: Math.round(THUMB_MAX_DIMENSION * widthHeightRatio), + height: THUMB_MAX_DIMENSION, + } + } else { + maxData = { + src: null, + width: THUMB_MAX_DIMENSION, + height: Math.round(THUMB_MAX_DIMENSION / widthHeightRatio), + } + } + + return { + preparedData, + originalData, + maxData, + } + } + private convertLazyLoadToImagesImpl(doc: DominoElement) { const protocol = 'https://' const spans = doc.querySelectorAll('.pcs-lazy-load-placeholder') spans.forEach((span: DominoElement) => { - // Create a new img element - const img = doc.createElement('img') as DominoElement + const { preparedData, originalData, maxData } = this.calculateImageDimensions(span) - // Set the attributes for the img element based on the data attributes in the span - - // The data-data-file-original-src attribute is the URL of the image that was used in the original article. - // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up in order to - // be "full width" on mobile devices. However, if the mobile API didn't scale the image up, then the - // data-data-file-original-src attribute will be missing, and we should use the data-src. - // See https://github.com/openzim/mwoffliner/issues/1925. - let originalWidth: number - let match: RegExpMatchArray | undefined - const originalSrc = span.getAttribute('data-data-file-original-src') - if (originalSrc) { - // Try to match against an image URL with a width in it. - match = THUMB_WIDTH_REGEX.exec(originalSrc) - if (match) { - originalWidth = parseInt(match[1], 10) - } + const widthToData = { + [preparedData.width]: preparedData, + [maxData.width]: maxData, + [originalData?.width || 0]: originalData, } - // These are the attributes that were "prepared" for us by the mobile-html endpoint. - const preparedSrc = span.getAttribute('data-src') - const preparedWidth = parseInt(span.getAttribute('data-width') || '0', 10) - const preparedHeight = parseInt(span.getAttribute('data-height') || '0', 10) - - let imgSrc = preparedSrc - let width = preparedWidth - if (originalWidth && match && originalWidth < preparedWidth) { - // There was a match on the originalSrc, and it is an image that is smaller than the prepared image. - width = originalWidth - imgSrc = originalSrc - } - if (THUMB_MAX_WIDTH < originalWidth || (!originalWidth && THUMB_MAX_WIDTH < preparedWidth)) { - // If both srcs are too big, try to either use the original URL hacking, or URL hacking on the - // "prepared" src, to get an image of the right size. - let srcToReplace = originalSrc - if (!match) { - // Try to match against the prepared URL, it might have sizing information. - match = THUMB_WIDTH_REGEX.exec(preparedSrc) - srcToReplace = preparedSrc + const minWidth = originalData ? Math.min(preparedData.width, maxData.width, originalData?.width) : Math.min(preparedData.width, maxData.width) + let selectedData = widthToData[minWidth] + + if (selectedData === maxData) { + // We've decided to scale down the image. Use URL hacking to create an image that scales to the size we want. + if (originalData) { + const match = THUMB_WIDTH_REGEX.exec(originalData.src) + if (match) { + selectedData.src = originalData.src.replace(`${match[1]}px`, `${selectedData.width}px`) + } + } else { + // No original src, or original src cannot be URL hacked. + const match = THUMB_WIDTH_REGEX.exec(preparedData.src) + if (match) { + selectedData.src = preparedData.src.replace(`${match[1]}px`, `${selectedData.width}px`) + } } - // If there is no match, we will just use the prepared image as is. - if (match) { - width = THUMB_MAX_WIDTH - imgSrc = srcToReplace.replace(`${match[1]}px`, `${width}px`) + } + + if (selectedData.src === null) { + // We couldn't find a URL to hack, so use the smaller of the original or prepared data. + if (!originalData) { + selectedData = preparedData + } else { + const newMinWidth = Math.min(preparedData.width, originalData.width) + selectedData = widthToData[newMinWidth] } } - // If the above ifs didn't execute, we're using the prepared image. - // This is a no-op if width == preparedWidth. - const height = Math.round((preparedHeight * width) / preparedWidth) - img.src = urlJoin(protocol, imgSrc) + // Create a new img element + const img = doc.createElement('img') as DominoElement + img.src = urlJoin(protocol, selectedData.src) img.setAttribute('decoding', 'async') - img.width = width - img.height = height + img.width = selectedData.width + img.height = selectedData.height img.className = span.getAttribute('data-class') // Replace the span with the img element @@ -213,7 +260,7 @@ export class WikimediaMobileRenderer extends MobileRenderer { } public readonly INTERNAL = { - convertLazyLoadToImages: this.convertLazyLoadToImagesImpl, - unhideSections: this.unhideSectionsImpl, + convertLazyLoadToImages: this.convertLazyLoadToImagesImpl.bind(this), + unhideSections: this.unhideSectionsImpl.bind(this), } } diff --git a/test/unit/renderers/mobile.renderer.test.ts b/test/unit/renderers/mobile.renderer.test.ts index 34b220e7..501e91d2 100644 --- a/test/unit/renderers/mobile.renderer.test.ts +++ b/test/unit/renderers/mobile.renderer.test.ts @@ -122,123 +122,246 @@ describe('mobile renderer', () => { expect(imgs[1].height).toEqual(167) }) - test('uses max width of 320 when src and data-data-file-original-src are both bigger', async () => { - const test_window = domino.createWindow( - ` - { + test('uses max width of 320 when src and data-data-file-original-src are both bigger', async () => { + const test_window = domino.createWindow( + ` + - `, - 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', - ) - const mobileRenderer = new WikimediaMobileRenderer() + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) - const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') - const imgs = actual.querySelectorAll('img') + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') - expect(spans.length).toBe(0) - expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') - expect(imgs[0].width).toEqual(320) - expect(imgs[0].height).toEqual(320) - }) - test('uses max width of 320 when there is no original src and the prepared src is too big', async () => { - const test_window = domino.createWindow( - ` - { + const test_window = domino.createWindow( + ` + - `, - 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', - ) - const mobileRenderer = new WikimediaMobileRenderer() + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) - const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') - const imgs = actual.querySelectorAll('img') + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') - expect(spans.length).toBe(0) - expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') - expect(imgs[0].width).toEqual(320) - expect(imgs[0].height).toEqual(320) - }) - test('uses original src width when it is the smallest', async () => { - const test_window = domino.createWindow( - ` - { + const test_window = domino.createWindow( + ` + - `, - 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', - ) - const mobileRenderer = new WikimediaMobileRenderer() + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) - const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') - const imgs = actual.querySelectorAll('img') + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') - expect(spans.length).toBe(0) - expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/150px-BMW.svg.png') - expect(imgs[0].width).toEqual(150) - expect(imgs[0].height).toEqual(150) - }) - test('uses prepared src when there is no original src, and no way to URL hack', async () => { - const test_window = domino.createWindow( - ` - { + const test_window = domino.createWindow( + ` + - `, - 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', - ) - const mobileRenderer = new WikimediaMobileRenderer() + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() - const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) - const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') - const imgs = actual.querySelectorAll('img') + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') - expect(spans.length).toBe(0) - expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/BMW.svg.png') - expect(imgs[0].width).toEqual(800) - expect(imgs[0].height).toEqual(800) + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/BMW.svg.png') + expect(imgs[0].width).toEqual(800) + expect(imgs[0].height).toEqual(600) + }) + }) + + describe('when the image needs to be scaled in the height dimension', () => { + test('uses max height of 320 when src and data-data-file-original-src are both bigger', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/256px-BMW.svg.png') + expect(imgs[0].width).toEqual(256) + expect(imgs[0].height).toEqual(320) + }) + test('uses max height of 320 when there is no original src and the prepared src is too big', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/256px-BMW.svg.png') + expect(imgs[0].width).toEqual(256) + expect(imgs[0].height).toEqual(320) + }) + test('uses the prepared src width when it is the smallest', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/120px-BMW.svg.png') + expect(imgs[0].width).toEqual(120) + expect(imgs[0].height).toEqual(150) + }) + test('uses prepared src when there is no original src, and no way to URL hack', async () => { + const test_window = domino.createWindow( + ` + + `, + 'http://en.wikipedia.org/api/rest_v1/page/mobile-html/BMW', + ) + const mobileRenderer = new WikimediaMobileRenderer() + + const actual = mobileRenderer.INTERNAL.convertLazyLoadToImages(test_window.document) + const spans = actual.querySelectorAll('.pcs-lazy-load-placeholder') + const imgs = actual.querySelectorAll('img') + + expect(spans.length).toBe(0) + expect(imgs.length).toBe(1) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/BMW.svg.png') + expect(imgs[0].width).toEqual(1200) + expect(imgs[0].height).toEqual(1500) + }) }) }) }) From a88ff8837dcc74c2df26e355e313d2a8c88699ea Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Tue, 19 Nov 2024 13:07:43 -0800 Subject: [PATCH 5/6] Flip the logic, the smaller dimension should be restricted --- src/renderers/wikimedia-mobile.renderer.ts | 2 +- test/unit/renderers/mobile.renderer.test.ts | 32 ++++++++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index 2fbf866e..0ff71963 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -112,7 +112,7 @@ export class WikimediaMobileRenderer extends MobileRenderer { // Calculate the ratio so we know if we're scaling down in the width or height dimension. const widthHeightRatio = preparedData.width / preparedData.height - let scaleUsingHeight = widthHeightRatio < 1.0 + let scaleUsingHeight = widthHeightRatio > 1.0 // The data-data-file-original-src attribute is the URL of the image that was used in the original article. // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up in order to diff --git a/test/unit/renderers/mobile.renderer.test.ts b/test/unit/renderers/mobile.renderer.test.ts index 501e91d2..f0238c1f 100644 --- a/test/unit/renderers/mobile.renderer.test.ts +++ b/test/unit/renderers/mobile.renderer.test.ts @@ -122,8 +122,8 @@ describe('mobile renderer', () => { expect(imgs[1].height).toEqual(167) }) - describe('when the image needs to be scaled in the width dimension', () => { - test('uses max width of 320 when src and data-data-file-original-src are both bigger', async () => { + describe('when the image width is greater than the height', () => { + test('uses max height of 320 when src and data-data-file-original-src are both bigger', async () => { const test_window = domino.createWindow( ` { expect(spans.length).toBe(0) expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') - expect(imgs[0].width).toEqual(320) - expect(imgs[0].height).toEqual(256) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/400px-BMW.svg.png') + expect(imgs[0].width).toEqual(400) + expect(imgs[0].height).toEqual(320) }) - test('uses max width of 320 when there is no original src and the prepared src is too big', async () => { + test('uses max height of 320 when there is no original src and the prepared src is too big', async () => { const test_window = domino.createWindow( ` { expect(spans.length).toBe(0) expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') - expect(imgs[0].width).toEqual(320) - expect(imgs[0].height).toEqual(256) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/400px-BMW.svg.png') + expect(imgs[0].width).toEqual(400) + expect(imgs[0].height).toEqual(320) }) test('uses the prepared src width when it is the smallest', async () => { const test_window = domino.createWindow( @@ -243,7 +243,7 @@ describe('mobile renderer', () => { }) }) - describe('when the image needs to be scaled in the height dimension', () => { + describe('when the image height is greater than the width', () => { test('uses max height of 320 when src and data-data-file-original-src are both bigger', async () => { const test_window = domino.createWindow( ` @@ -270,9 +270,9 @@ describe('mobile renderer', () => { expect(spans.length).toBe(0) expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/256px-BMW.svg.png') - expect(imgs[0].width).toEqual(256) - expect(imgs[0].height).toEqual(320) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') + expect(imgs[0].width).toEqual(320) + expect(imgs[0].height).toEqual(400) }) test('uses max height of 320 when there is no original src and the prepared src is too big', async () => { const test_window = domino.createWindow( @@ -299,9 +299,9 @@ describe('mobile renderer', () => { expect(spans.length).toBe(0) expect(imgs.length).toBe(1) - expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/256px-BMW.svg.png') - expect(imgs[0].width).toEqual(256) - expect(imgs[0].height).toEqual(320) + expect(imgs[0].src).toEqual('https://upload.wikimedia.org/wikipedia/commons/thumb/4/44/BMW.svg/320px-BMW.svg.png') + expect(imgs[0].width).toEqual(320) + expect(imgs[0].height).toEqual(400) }) test('uses the prepared src width when it is the smallest', async () => { const test_window = domino.createWindow( From 3d237e727ab127b2ba1f7db47160dff914e8b119 Mon Sep 17 00:00:00 2001 From: Travis Briggs Date: Tue, 19 Nov 2024 13:11:37 -0800 Subject: [PATCH 6/6] Fix lint --- src/renderers/wikimedia-mobile.renderer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/wikimedia-mobile.renderer.ts b/src/renderers/wikimedia-mobile.renderer.ts index 0ff71963..b33fd3f9 100644 --- a/src/renderers/wikimedia-mobile.renderer.ts +++ b/src/renderers/wikimedia-mobile.renderer.ts @@ -112,7 +112,7 @@ export class WikimediaMobileRenderer extends MobileRenderer { // Calculate the ratio so we know if we're scaling down in the width or height dimension. const widthHeightRatio = preparedData.width / preparedData.height - let scaleUsingHeight = widthHeightRatio > 1.0 + const scaleUsingHeight = widthHeightRatio > 1.0 // The data-data-file-original-src attribute is the URL of the image that was used in the original article. // It is preferred over the data-src attribute, which is a "mobile" image that may be scaled up in order to