Bug 85874 – register-protocol-handler-expected.txt seems wrong.

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Dongwoo Joshua Im (dwim)]

IMHO, LayoutTests/fast/dom/register-protocol-handler-expected.txt seems wrong.

LayoutTests/fast/dom/register-protocol-handler-expected.txt should be same as LayoutTests/platform/chromium/fast/dom/register-protocol-handler-expected.txt.

[Dongwoo Joshua Im (dwim)]

Created attachment 140693 [details]
Patch

[Dongwoo Joshua Im (dwim)]

I think LayoutTests/fast/dom/register-protocol-handler-expected.txt should have positive test result, 
so that the ports which not support this feature would fail this test case.

[Ben Wells]

I don't understand. It is confusing, but a test pass for ports which don't support rph is to get the failures reported. If a port does not support rph it is expected and desired to get these failures.

Are you suggesting to add in test_expectations or skips? Currently this test is checking that rph is successfully disabled where it should be, so skipping or expecting failure would be a loss of coverage.

[Dongwoo Joshua Im (dwim)]

(In reply to comment #3)
> I don't understand. It is confusing, but a test pass for ports which don't support rph is to get the failures reported. If a port does not support rph it is expected and desired to get these failures.
> 
> Are you suggesting to add in test_expectations or skips? Currently this test is checking that rph is successfully disabled where it should be, so skipping or expecting failure would be a loss of coverage.

IMO, the ports which are not supported the registerProtocolHandler API should add this test case into Skipped file, or fail this case.

As I know, that is how WebKit Layouttest works... isn't it?


If needed, I can fix Skipped file of all of the ports which are not supported the registerProtocolHandler API in this bug as a new patch.

[Ben Wells]

If a port does not support rph, and a site tries to use it, it should give an error. That is the correct behaviour, and it is being tested by this test.

By skipping the test, the test coverage will be reduced. If anyone accidentally enabled rph on a port it would not be noticed.

Perhaps to reduce confusion you could update the test to not say 'pass / fail' everywhere.

[Ben Wells]

BTW The first line of the expected result makes it clear:
"This test makes sure that navigator.registerProtocolHandler throws the proper exceptions and has no-op default implementation."

Maybe this should be updated to say what should happen if registerProtocolHandler is supported.

[Dongwoo Joshua Im (dwim)]

(In reply to comment #6)
> BTW The first line of the expected result makes it clear:
> "This test makes sure that navigator.registerProtocolHandler throws the proper exceptions and has no-op default implementation."
> 
> Maybe this should be updated to say what should happen if registerProtocolHandler is supported.

Actually, I'm implementing the rph on EFL port, so it becomes issue for me.

I'll just add the expected result on EFL port. (same expected result with chromium port)

[Dongwoo Joshua Im (dwim)]

Regarding the Ben's opinion,
I want to close this issue.