938699 - Kill off nsDOMWindowUtils::FindElementWithViewId

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Description

[Kartikaya Gupta (email:[e-mail])]

This function is only used in one place, and that can be easily fixed. The reason I want to get rid of this function is that it's a footgun because it special-cases the ROOT_SCROLL_ID and can return a different value from nsLayoutUtils::FindIDFor for it.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

This function isn't used any more, it's should be ok to just delete it from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp. It's a simple first bug, I can mentor it. The only 'trick' here is to bump the UUID in the IDL file.

Comment 2

[Jeffrey Tran]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #1)
> This function isn't used any more, it's should be ok to just delete it from
> nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp. It's a simple first bug, I
> can mentor it. The only 'trick' here is to bump the UUID in the IDL file.

Hi, can you explain how to "bump the UUID"?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

The easiest way is to run |mach uuid| to generate a new UUID and replace the existing one (e.g. at [1]) with the new one.

[1] https://dxr.mozilla.org/mozilla-central/rev/aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/dom/interfaces/base/nsIDOMWindowUtils.idl#52

Comment 4

[Kartikaya Gupta (email:[e-mail])]

.. although I think you might not need to do that anymore, according to the discussion at https://groups.google.com/forum/#!topic/mozilla.dev.platform/n6qBpyxoI6I

Comment 5

[Jeffrey Tran]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #4)
> .. although I think you might not need to do that anymore, according to the
> discussion at
> https://groups.google.com/forum/#!topic/mozilla.dev.platform/n6qBpyxoI6I

So then do we just delete the function and not worry about the UUID?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Yup!

Comment 7

[Jeffrey Tran]

Attachment: rev1 - Remove FindElementWithViewId from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8716417 [details] [diff] [review]
rev1 - Remove FindElementWithViewId from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp.

Review of attachment 8716417 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks! I'll land this in a bit.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00b5b6c9f887

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/00b5b6c9f887

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Jeffrey, thank you for the patch!

Comment 12

[Jeffrey Tran]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> Jeffrey, thank you for the patch!

I'm happy to help!