Correctly scroll the search result in the viewport with rotated pdfs (bug 2021392)

[calixteman]

No description provided.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.60%. Comparing base (0a6894d) to head (2436593).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #20951      +/-   ##
==========================================
- Coverage   62.61%   62.60%   -0.02%     
==========================================
  Files         174      174              
  Lines      121951   121951              
==========================================
- Hits        76364    76349      -15     
- Misses      45587    45602      +15     

Flag	Coverage Δ
fonttest	?
unittestcli	62.60% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[timvandermeij]

r=me, with passing integration tests. Thank you for fixing this!

[timvandermeij]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 1

Live output at: http://54.241.84.105:8877/2883fd1edc9c8d5/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 1

Live output at: http://54.193.163.58:8877/21533ebc43d36ec/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/2883fd1edc9c8d5/output.txt

Total script time: 26.10 mins

• Integration Tests: Passed

[Snuffleupagus]

A couple of questions:

• Are there any remaining callers of the scrollIntoView helper function that pass in scrollMatches = true now, because if not that code can probably be simplified?
  

  pdf.js/web/ui_utils.js

  Lines 74 to 131 in 0a6894d

  	/**
  	* Scrolls specified element into view of its parent.
  	* @param {HTMLElement} element - The element to be visible.
  	* @param {Object} [spot] - An object with optional top and left properties,
  	* specifying the offset from the top left edge.
  	* @param {number} [spot.left]
  	* @param {number} [spot.top]
  	* @param {boolean} [scrollMatches] - When scrolling search results into view,
  	* ignore elements that either: Contains marked content identifiers,
  	* or have the CSS-rule `overflow: hidden;` set. The default value is `false`.
  	*/
  	function scrollIntoView(element, spot, scrollMatches = false) {
  	// Assuming offsetParent is available (it's not available when viewer is in
  	// hidden iframe or object). We have to scroll: if the offsetParent is not set
  	// producing the error. See also animationStarted.
  	let parent = element.offsetParent;
  	if (!parent) {
  	console.error("offsetParent is not set -- cannot scroll");
  	return;
  	}
  	let offsetY = element.offsetTop + element.clientTop;
  	let offsetX = element.offsetLeft + element.clientLeft;
  	while (
  	(parent.clientHeight === parent.scrollHeight &&
  	parent.clientWidth === parent.scrollWidth) ||
  	(scrollMatches &&
  	(parent.classList.contains("markedContent") ||
  	getComputedStyle(parent).overflow === "hidden"))
  	) {
  	offsetY += parent.offsetTop;
  	offsetX += parent.offsetLeft;
  	parent = parent.offsetParent;
  	if (!parent) {
  	return; // no need to scroll
  	}
  	}
  	if (spot) {
  	if (spot.top !== undefined) {
  	offsetY += spot.top;
  	}
  	if (spot.left !== undefined) {
  	if (scrollMatches) {
  	const elementWidth = element.getBoundingClientRect().width;
  	const padding = MathClamp(
  	(parent.clientWidth - elementWidth) / 2,
  	20,
  	400
  	);
  	offsetX += spot.left - padding;
  	} else {
  	offsetX += spot.left;
  	}
  	parent.scrollLeft = offsetX;
  	}
  	}
  	parent.scrollTop = offsetY;
  	}

• Does this patch perhaps also fix any of the following bugs/issues?
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1735730
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1808983
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1872667
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1972005
  • [Bug]: search / find - bad scroll position, weird stepping #18769

[moz-tools-bot]

From: Bot.io (Windows)

---

Success

Full output at http://54.193.163.58:8877/21533ebc43d36ec/output.txt

Total script time: 45.43 mins

• Integration Tests: Passed

[calixteman]

/botio-linux integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/e9a3f910ea23886/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/e9a3f910ea23886/output.txt

Total script time: 25.56 mins

• Integration Tests: Passed