third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https.html - Issue 2852423002: Expose passwords to JavaScript in Credential Manager API

Side by Side Diff: third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https.html

Issue 2852423002: Expose passwords to JavaScript in Credential Manager API (Closed)

Patch Set: WPT Tests and Readonly Password Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <script src=/resources/testharness.js></script>	2 <script src=/resources/testharness.js></script>

3 <script src=/resources/testharnessreport.js></script>	3 <script src=/resources/testharnessreport.js></script>

4 <script src=/resources/WebIDLParser.js></script>	4 <script src=/resources/WebIDLParser.js></script>

5 <script src=/resources/idlharness.js></script>	5 <script src=/resources/idlharness.js></script>

6 <script type="text/plain" id="untested">	6 <script type="text/plain" id="untested">

7 dictionary CredentialData {	7 dictionary CredentialData {

8 USVString id;	8 USVString id;

9 };	9 };

10	10

11 [Exposed=Window, SecureContext]	11 [Exposed=Window, SecureContext]

12 interface Credential {	12 interface Credential {

13 readonly attribute USVString id;	13 readonly attribute USVString id;

14 readonly attribute DOMString type;	14 readonly attribute DOMString type;

15 };	15 };

16	16

17 dictionary SiteBoundCredentialData : CredentialData {	17 dictionary SiteBoundCredentialData : CredentialData {
	Mike West 2017/05/08 11:01:57 We should have removed the `SiteBound` stuff in th We should have removed the `SiteBound` stuff in the other patch. Would you mind spinning off a new patch to clean up this test? jdoerrie 2017/05/08 14:03:17 Will do. I had another related question: In third_ Show quoted text On 2017/05/08 11:01:57, Mike West wrote: > We should have removed the `SiteBound` stuff in the other patch. Would you mind > spinning off a new patch to clean up this test? Will do. I had another related question: In third_party/WebKit/Source/modules/credentialmanager we use LocallyStoredCredentialData.idl which links to the (dead) https://w3c.github.io/webappsec/specs/credentialmanagement/#dictdef-locallyst.... Was this also part of the spec at some point and is now obsolete? It doesn't appear in this file, so my current assumption is that it's mainly an implementation detail at this point. Is this correct? If yes, should we keep it or also delete it? jdoerrie 2017/05/08 15:21:07 Nvm, I also deleted LocallyStoredCredentialData as Show quoted text On 2017/05/08 14:03:17, jdoerrie (slow this week) wrote: > On 2017/05/08 11:01:57, Mike West wrote: > > We should have removed the `SiteBound` stuff in the other patch. Would you > mind > > spinning off a new patch to clean up this test? > > Will do. I had another related question: In > third_party/WebKit/Source/modules/credentialmanager we use > LocallyStoredCredentialData.idl which links to the (dead) > https://w3c.github.io/webappsec/specs/credentialmanagement/#dictdef-locallyst.... > Was this also part of the spec at some point and is now obsolete? It doesn't > appear in this file, so my current assumption is that it's mainly an > implementation detail at this point. Is this correct? If yes, should we keep it > or also delete it? Nvm, I also deleted LocallyStoredCredentialData as part of the SiteBoundCredentialData cleanup: https://codereview.chromium.org/2867643004/ I will send that CL out for review by you when the bots are happy with it.
18 USVString name;	18 USVString name;

19 USVString iconURL;	19 USVString iconURL;

20 };	20 };

21	21

22 [NoInterfaceObject, SecureContext]	22 [NoInterfaceObject, SecureContext]

23 interface CredentialUserData {	23 interface CredentialUserData {

24 readonly attribute USVString name;	24 readonly attribute USVString name;

25 readonly attribute USVString iconURL;	25 readonly attribute USVString iconURL;

26 };	26 };

27	27

(...skipping 30 matching lines...) Expand all Loading...
58 };	58 };

59	59

60 [Constructor(PasswordCredentialData data),	60 [Constructor(PasswordCredentialData data),

61 Constructor(HTMLFormElement form),	61 Constructor(HTMLFormElement form),

62 Exposed=Window,	62 Exposed=Window,

63 SecureContext]	63 SecureContext]

64 interface PasswordCredential : Credential {	64 interface PasswordCredential : Credential {

65 attribute USVString idName;	65 attribute USVString idName;

66 attribute USVString passwordName;	66 attribute USVString passwordName;

67	67

68 attribute CredentialBodyType? additionalData;	68 attribute CredentialBodyType? additionalData;
	Mike West 2017/05/08 11:01:57 Please drop these three attributes from this test, Please drop these three attributes from this test, as it will be upstreamed and should reflect the state of the spec and not the state of Chrome. jdoerrie 2017/05/08 14:03:17 Will do this as well. Currently I am getting a pre Show quoted text On 2017/05/08 11:01:57, Mike West wrote: > Please drop these three attributes from this test, as it will be upstreamed and > should reflect the state of the spec and not the state of Chrome. Will do this as well. Currently I am getting a presubmit error after I removed the attributes and the appropriate lines from the -expected.txt file: * Presubmit ERRORS ** * The following files are passing testharness results without console error messages, they should be removed: //third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https-expected.txt ERROR: found passing testharness results without console error messages. From the looks of it it's complaining that there is no line with FAIL in that file anymore. The corresponding script (https://codesearch.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts...) states the following: """Check if a LayoutTest expected file is a passing testharness result. The intent of this script is to identify expected files that are passing testharness.js results. Those files are not needed because the test infrastructure will read the output of testharness.js tests if there is no expected files.""" Is it fine to simply go ahead and remove the *-expected.txt file? I don't fully understand this test framework yet, but removing the expectation file seems wrong to me. What do you think?
	69 readonly attribute DOMString password;

69 };	70 };

70 PasswordCredential implements CredentialUserData;	71 PasswordCredential implements CredentialUserData;

71	72

72 [Constructor(FederatedCredentialData data),	73 [Constructor(FederatedCredentialData data),

73 Exposed=Window,	74 Exposed=Window,

74 SecureContext]	75 SecureContext]

75 interface FederatedCredential : Credential {	76 interface FederatedCredential : Credential {

76 readonly attribute USVString provider;	77 readonly attribute USVString provider;

77 readonly attribute DOMString? protocol;	78 readonly attribute DOMString? protocol;

78 };	79 };

79 FederatedCredential implements CredentialUserData;	80 FederatedCredential implements CredentialUserData;

80 </script>	81 </script>

81 <script>	82 <script>

82 "use strict";	83 "use strict";

83 var idl_array = new IdlArray();	84 var idl_array = new IdlArray();

84 idl_array.add_untested_idls(document.querySelector('#untested').textContent) ;	85 idl_array.add_untested_idls(document.querySelector('#untested').textContent) ;

85 idl_array.add_idls(document.querySelector('#tested').textContent);	86 idl_array.add_idls(document.querySelector('#tested').textContent);

86 idl_array.add_objects({	87 idl_array.add_objects({

87 CredentialsContainer: ['navigator.credentials'],	88 CredentialsContainer: ['navigator.credentials'],

88 PasswordCredential: ['new PasswordCredential({ id: "id", password: "pencil ", iconURL: "https://example.com/", name: "name" })'],	89 PasswordCredential: ['new PasswordCredential({ id: "id", password: "pencil ", iconURL: "https://example.com/", name: "name" })'],

89 FederatedCredential: ['new FederatedCredential({ id: "id", provider: "http s://example.com", iconURL: "https://example.com/", name: "name" })']	90 FederatedCredential: ['new FederatedCredential({ id: "id", provider: "http s://example.com", iconURL: "https://example.com/", name: "name" })']

90 });	91 });

91 idl_array.test();	92 idl_array.test();

92 </script>	93 </script>

93	94

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https-expected.txt » ('j') | no next file with comments »