Closed Bug 1588791 Opened 2 years ago Closed 1 year ago

Fission iframes don't handle the scrolling="" attribute at all.

Categories

(Core :: DOM: Content Processes, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla74
Fission Milestone M6
Tracking Status
firefox74 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

<!doctype html>
<iframe src="https://crisal.io/tmp/file_bug590812.xml" scrolling="no"></iframe>
<iframe src="https://crisal.io/tmp/mask-image-resource.html" scrolling="yes"></iframe>

Both show scrollbars with Fission.

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies) → needinfo?(nkochar)
Fission Milestone: --- → ?
Flags: needinfo?(nkochar)
Priority: -- → P3
Fission Milestone: ? → M5
Flags: needinfo?(matt.woodrow)

Blocks enabling Fission in Nightly (M6).

Fission Milestone: M5 → M6

Once bug 1602322 lands, layout/reftests/scrolling/[i]frame-scrolling-attr-[1|2].html will have test coverage for this, and we'll be able to remove the fails-if(browserIsFission) annotation.

Flags: needinfo?(matt.woodrow)

Guess I can take this.

Assignee: nobody → emilio
Depends on: 1603889
Depends on: 1606659
Blocks: 1606660

Right now we do the same thing in two pretty different code paths... That's not
great, so unify them.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d59e691bda9e
Cleanup threading of OwnerShowInfo. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/52e661ff161d
Make fission iframes honor and deal with the scrolling attribute. r=mattwoodrow
Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4eb188ccbe0
Cleanup threading of OwnerShowInfo. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/05639d6591b3
Make fission iframes honor and deal with the scrolling attribute. r=mattwoodrow
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

Thanks Emilio! Looks like this fixed all but one of the failures in the layout/reftests/scrolling/[i]frame-scrolling-attr*

iframe-scrolling-attr-2.html seems to be still failing though. Linux reftests also got moved to ubuntu 18.04 since my last try push, so that might be another variable affecting this.

Here is a link to a failure

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=89049222cfae325c46eef51c27bf4bc9a4b18aff&selectedJob=284999905

Emilio, could you take a look and decide if we need a new bug to fix this?

Flags: needinfo?(emilio)

The test works, but it expects the change to be synchronous, which for fission iframe it's not.

So that test is racy, gotta figure out how to wait for the message to be processed or something.

Tim, do you know any easy way to wait until all the relevant IPC messages have been processed? Otherwise I guess I'll just rewrite the test to use postMessage or something.

Flags: needinfo?(emilio) → needinfo?(tnikkel)

Hmm, I do not. I think it might be a wider issue, I'm debugging a different reftest fission failure and I don't see anything that will guarantee an EffectsInfo ipc message will be delivered before the final snapshot. So we might need to figure something out more general.

Flags: needinfo?(tnikkel)
Regressions: 1624747
You need to log in before you can comment on or make changes to this bug.