private method can be redeclared in a child class as public without the compiler fail to compile

[carlosrovira]

Hi,

working in an example for Tour De Jewel found the following bug:

• In 'components.ExampleAndSourceCodeTabbedSectionContent.as' I have a private 'changeHandler' method.
• In 'ButtonBarPlayGround.mxml' (that extends 'ExampleAndSourceCodeTabbedSectionContent') I created a public 'changeHandler' method that is called from a ButtonBar instance in the 'change' event declared in mxml.

Running TDJ result in the following issue: 'change' event handler tries to call the private changeHandler in 'ExampleAndSourceCodeTabbedSectionContent' and fails with an error since event target is not the right one.

This seems to me a bug since compiler should fail to compile and report that 'changeHandler' method is already declared in the parent class as private. Or maybe I'm wrong?

[aharui]

Which code called addEventListener?

Try it in Flex and see what happen

[carlosrovira]

HI Alex,

this is the addEventListener line: https://github.com/apache/royale-asjs/blob/b165d70107414be6ab18696c9b4bf2a7b5583bfb/examples/royale/TourDeJewel/src/main/royale/components/ExampleAndSourceCodeTabbedSectionContent.as#L90

I don't have currently a Flex environment available. If others can try it will be of help

Thanks

[joshtynjala]

According to Adobe's ActionScript Compiler 2.0 Backward Compatibility document, it is intentional to "allow multiple definitions with the same unqualified name as long as they have disjoint namespaces".

[carlosrovira]

Hi Josh, the example you mention refers to a class, and classes can be in different namespaces.
But in this case, I'm talking about methods in the same class, and that methods are in the same namespace.

A part from that, it seems that allowing that kind of behavior introduces confusion for the user (and that's what we want to avoid by using a compiler) and does not seems to solve a valid use case in the real world.

just my 2 :)

[joshtynjala]

the example you mention refers to a class

I'll copy the quote here again and add emphasis: "allow multiple definitions with the same unqualified name as long as they have disjoint namespaces"

It specifically says "definitions". The classes there are just an example. Methods and variables are definitions too.

I'm talking about methods in the same class, and that methods are in the same namespace.

You are talking about methods in two classes (a subclass and a superclass), and they have different namespaces (one is public and the other is private).

Now, I'm not saying that we don't have a bug. It's just that the bug is that the wrong method is being called rather than the fact that there are two methods with the same name and different namespaces. In this case, the private method isn't supposed to be accessible to the subclass at all. That's why there are no compiler errors. The compiler knows that only the public method should be accessible in the subclass. The private method would need to be protected for the compiler to report an ambiguous reference error.

With that in mind, the fix is something along the lines of determining why the wrong method is being chosen by the JS emitter and choosing the correct one instead.

[aharui]

Please attach ButtonBarPlayGround.mxml and the transpiled ButtonBarPlayGround.js
In theory, the addEventListener you linked to will result in the private changeHandler getting called when the TabBar changes. I think you are saying that some other thing dispatches a change event and results in the private changeHandler being called.

[carlosrovira]

Here's the ButtonBarPlayGround.mxml modified to show the error (the actual one in repo is done to avoid name collisions. This is the corresponding js after compilation.

This is the error in browser console:

ExampleAndSourceCodeTabbedSectionContent.as:121 Uncaught TypeError: Cannot read property 'selectedItem' of null
at ButtonBarPlayGround.components.ExampleAndSourceCodeTabbedSectionContent.components_ExampleAndSourceCodeTabbedSectionContent_changeHandler (ExampleAndSourceCodeTabbedSectionContent.as:121)
at ButtonBarPlayGround.$EH8 (ButtonBarPlayGround.mxml:146)
at Function.goog.events.fireListener [as googFireListener] (events.js:744)
at Function.org.apache.royale.core.HTMLElementWrapper.fireListenerOverride [as googFireListener] (HTMLElementWrapper.as:170)
at Object.org.apache.royale.core.ElementWrapper.fireListenerOverride [as fireListener] (ElementWrapper.as:188)
at HTMLDivElement.goog.events.handleBrowserEvent_ (events.js:870)
at HTMLDivElement.f (events.js:289)
at org.apache.royale.jewel.ToggleButtonBar.org.apache.royale.core.HTMLElementWrapper.dispatchEvent (HTMLElementWrapper.as:401)
at org.apache.royale.jewel.beads.controllers.ListSingleSelectionMouseController.selectedHandler (ListSingleSelectionMouseController.as:151)
at org.apache.royale.jewel.itemRenderers.ToggleButtonBarItemRenderer.goog.events.EventTarget.fireListeners (eventtarget.js:284)

[aharui]

The JS file looks correct to me, and I do not understand how it could call
ExampleAndSourceCodeTabbedSectionContent_changeHandler. Please set a breakpoint on ButtonBarPlayground.$EH8 and step through to see how it ends up in that method.

[joshtynjala]

I've determined the cause of this issue. The emitter is outputting the following code when using addEventListener() in ExampleAndSourceCodeTabbedSectionContent:

org.apache.royale.utils.Language.closure(this.components_ExampleAndSourceCodeTabbedSectionContent_changeHandler, this, 'changeHandler')

The function name in the final parameter is wrong. It should be "components_ExampleAndSourceCodeTabbedSectionContent_changeHandler" instead. Because it is using the wrong name there, it's overwriting the public function from the subclass.

[carlosrovira]

Great you catch it Josh! :)