nsIPresShell::RENDER_IGNORE_VIEWPORT_SCROLLING documentation is misleading.

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jib, Assigned: tnikkel)

Tracking

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

Attachments

(1 attachment)

STR: See Bug 1193075 comment 32.

Basically, user-scrolling still registers in the demo shown there, even though the scrollWithPage is absent/false which sets the  nsIPresShell::RENDER_IGNORE_VIEWPORT_SCROLLING flag.

Not quite sure what the right component or who the right person is for this bug.
(Assignee)

Comment 1

3 years ago
As currently implemented in the code it looks like the effect of RENDER_IGNORE_VIEWPORT_SCROLLING is to:
-not draw scrollbars
-not clip based on the document viewport
Comments in the code that say it draws as if the document was not scrolled at all do not match what the code implements.

If one adds the RENDER_DOCUMENT_RELATIVE flag then in addition to treating the rect passed in as being relative to the document (and not the viewport) the code will also draw as if there was no scrolling in the document. The comments for this flag seem to also not describe what is implemented.

So I think if you add the RENDER_DOCUMENT_RELATIVE flag you should hopefully get the result you want.

I have no idea about the history of these flags and what we implemented and why and what dependencies there may be. Perhaps the easiest fix is to just rename the flags to correspond to what the code currently does, and adjust the comments accordingly. Although, perhaps the current code got wrongly changed at some point and the initial code the implement the flags matched the comments and there was a good reason for it being that way.
That worked, thanks!
Still relevant?
Flags: needinfo?(jib)
(In reply to Timothy Nikkel (:tn) from comment #1)
> Comments in the code that say it draws as if the document was not scrolled
> at all do not match what the code implements.
[...]
> The comments for this flag seem to also not describe what is implemented.
[...]
> Perhaps the easiest fix is to just rename the flags to correspond to what
> the code currently does, and adjust the comments accordingly.

Seems like at least the comments could be improved.
Flags: needinfo?(jib)
Summary: nsIPresShell::RENDER_IGNORE_VIEWPORT_SCROLLING doesn't seem to work. → nsIPresShell::RENDER_IGNORE_VIEWPORT_SCROLLING documentation is misleading.
:tn - tentatively moved this to layout; please revise if that's wrong.  It's not WebRTC.  Also, if you can CC or NI whomever should look at it, that would be good.  Thanks
Component: WebRTC → Layout
Flags: needinfo?(tnikkel)
(Assignee)

Comment 6

3 years ago
Created attachment 8683475 [details] [diff] [review]
Fix the comments.

"ignore viewport scrolling" is used in a lot of places, not just this flag. I didn't want to update them all. (Or come up with better names.)
Assignee: nobody → tnikkel
Flags: needinfo?(tnikkel)
Attachment #8683475 - Flags: review?(roc)

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/747949534cf8
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.