Open Bug 1685328 Opened 4 years ago Updated 4 years ago

Print preview page view count is not reset when modifying the margins value

Categories

(Core :: Print Preview, defect, P4)

Desktop
All
defect

Tracking

()

Tracking Status
firefox-esr78 --- unaffected
firefox84 --- unaffected
firefox85 --- wontfix
firefox86 --- wontfix
firefox87 --- fix-optional

People

(Reporter: csasca, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [print2021_v86] [old-ui-])

Affected versions

  • Firefox 85.0b5
  • Firefox 86.0a1

Affected platforms

  • macOS 10.15.7
  • Windows 10

Steps to reproduce

  1. Launch Firefox
  2. Access this example page
  3. Print preview it and go to the last page in preview
  4. Select more settings and modify the "margins" value

Expected result

  • Margins are modified, and the print preview page count is reset

Actual result

  • The print preview page view count is still the same as before (last page), but the actual page in preview is the first one

Regression range

  • Will see for a regression.

Additional notes

  • The issue can be seen in the following attachment
  • Ubuntu 20.04 doesn't seem to be affected
OS: macOS → All

This bug is basically not a regression because the it occurs since the implementation of the hovering page scroll buttons (the buttons that appear when scrolling on top of the print preview).

Mozregression results:

"
2021-01-07T11:20:22: DEBUG : Found commit message:
Bug 1675360 - Shutdown MediaKeys when parent inner window is destroyed rather than document. r=karlt

Rework the MediaKeys class to shutdown when its parent inner window is destroyed
rather than the document it's created in. This is done to mitigate the case
where a MediaKeys is created in an about:blank document that has not yet
undergone its async load (i.e. blank document that will stay blank, blank
documents loading to other pages will still clobber their keys on load). This
specifically addresses cases where sites could create an iframe, not wait for
load, create a MediaKeys in the iframe, and then find the keys had become
unusable.

This is achieved by associating MediaKeys instances with their inner window and
having the window notify keys when a window is going to be destroyed. I decided
to use this approach rather than have MediaKeys observe for window destruction
for a few reasons:

  • The keys would need to support weak references to use the observer service in
    the desired way. Implementing this interface on the MediaKeys adds a
    non-trivial level of complexity and means the keys would implement the WeakPtr
    interface (already in place prior to this patch) and also the NS weak
    reference interface, which I found confusing.
  • If the inner window stores pointers to MediaKeys created in it, it can be self
    aware of if EME activity is taking place within it. The allows us to better
    identify EME activity in documents. Historically one of the ways we'd
    determined EME activity by checking if media elements have MediaKeys attached,
    but this had lead to issues where if MediaKeys are created but not attached
    to a media element we overlook them. With this patch's changes, we can also
    have a document check its inner window to see if there are any MediaKeys. This
    patch uses this to extend our check to avoid bfcaching pages with EME content.
  • There appears to be prior art using a similar approach for audio contexts and
    peer connections, which I assume is sensible and I'm not reinventing the wheel
    by following.
    "

Bisection continued; end results:

"
2021-01-07T11:29:26: DEBUG : Found commit message:
Bug 1654684 - Add sheet-indicator and preview navigation to the print preview. r=mstriemer,fluent-reviewers,flod

Differential Revision: https://phabricator.services.mozilla.com/D96737

2021-01-07T11:29:26: DEBUG : Did not find a branch, checking all integration branches
2021-01-07T11:29:26: INFO : The bisection is done.
2021-01-07T11:29:26: INFO : Stopped
"

Has Regression Range: --- → yes
Has STR: --- → yes

I can reproduce the STR in Nightly on Linux. I noted that if I change the margin to a large number (say 3) so that the number of total pages changes, then both the current page and total page numbers are updated correctly.

I wonder what the desired behavior is though... It seems to me that the root of the problem is that we reset the scroll position to the start of the document. I think we should try to keep the scroll view at the current document position as much as possible, although that's probably not a panacea since I suspect the current page number might change in some cases, so we probably need some "sync page number to scroll position"-mechanism anyway.

BTW, the same bug also occurs when changing Scale from 100 to 90.

Priority: -- → P4
You need to log in before you can comment on or make changes to this bug.