Closed Bug 1609426 Opened 4 years ago Closed 4 years ago

Some DOM methods don't work with eager evaluation

Categories

(DevTools :: Console, enhancement, P2)

enhancement

Tracking

(firefox75 verified)

VERIFIED FIXED
Firefox 75
Tracking Status
firefox75 --- verified

People

(Reporter: Harald, Assigned: loganfsmyth)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files, 1 obsolete file)

What were you doing?

  1. Type document.getElementsByName('div') in Console

What happened?

No result preview.

What should have happened?

Eager eval works.

Anything else we should know?

Exploring the DOM is one of the most common examples used when showing off.

See Also: → 1607608

Logan, can you help me understand if we need help from the DOM team to get those enabled or is there a whitelist that we can extend?

Flags: needinfo?(loganfsmyth)

I don't think we need help from the DOM team, but we'll have to come up with a list of API functions that we support. So far we have a whitelist of all of the standard ECMAScript language functions that I assume Brian could come up with: https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/devtools/server/actors/webconsole/eval-with-debugger.js#332-458 So we'll need to come up with what new entries we want to add to that list for functions from other specifications.

Flags: needinfo?(loganfsmyth)

Emilio was saying we could look into extracting the [Pure] keywords from .webidl files.
Not sure how we could that, but we should check what's our options there.

Ah I wasn't aware of that annotation. I could certainly see that being useful as a way to build up the list for us.

Blocks: 1612560

If we decide to go the Pure route, we should spin out a separate bug for non-Pure but side-effect free methods like document.getSelection from bug 1612560.

Assignee: nobody → loganfsmyth
Status: NEW → ASSIGNED

If we decide to go the Pure route, we should spin out a separate bug for non-Pure but side-effect free methods

If you find such methods, they should just get marked [Pure].

If you find such methods, they should just get marked [Pure].

Some are mostly-pure-with-caveats, like String.match (see bug 1609429); but Logan also found DOM methods that should be pure but are not marked as such – we will file those.

Pushed by loganfsmyth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/60b53a2f8f19
Part 1: Allow CCW natives for isSameNative. r=arai
https://hg.mozilla.org/integration/autoland/rev/05583692c421
Part 2: Expose more possible Sandbox bindings. r=bzbarsky

@Harald Are you fine with the pure-ewith-caveats approach? bz raised an interesting case in https://phabricator.services.mozilla.com/D61966#inline-376904 around triggering layout, and I'm not sure how far we want to go with allowing side-effects for things like that.

Flags: needinfo?(hkirschner)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75

@Harald Are you fine with the pure-ewith-caveats approach? bz raised an interesting case in https://phabricator.services.mozilla.com/D61966#inline-376904 around triggering layout, and I'm not sure how far we want to go with allowing side-effects for things like that.

On the benefits side, size measurements are useful for developers exploring a page. If I understand the side-effect correctly: triggering layout is mostly a problem in a code flow that flushes pending CSS changes. Side-effect free should not update styling, so forcing layout does not seem an obvious issue.

I am OK with having those in our side-effect-free-ish list (including String::match) and revisit if we get bug reports.

Flags: needinfo?(hkirschner)

Woops, only some of these patches landed so this is not done yet.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 1616013
Pushed by loganfsmyth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/48bead797c47
Part 3: Automatically generate the list of pure functions. r=jlast
https://hg.mozilla.org/integration/autoland/rev/1d24a1f7cf64
Part 4: Allow native DOM bindings during eager eval. r=jlast,bzbarsky
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Attachment #9126306 - Attachment is obsolete: true
Flags: qe-verify+

Verified with 75.0b6 on Windows 10, macOS 10.15, Ubuntu 19.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: