Implementing activation behavior for <label> #8260

martiansideofthemoon · 2015-10-30T12:30:48Z

Attempt to resolve #8179
@Manishearth , could you give me some resources having more information about what each function in Activatable does? The code compiles on my machine but I guess a lot more is needed

ManishBot · 2015-10-30T13:09:13Z

@bors-servo try

(ignore me, and ignore the test failure that's going to happen below, that will be caused by me canceling the build)

bors-servo · 2015-10-30T13:09:16Z

⌛ Trying commit 19e1a0d with merge ce764be...

@Manishearth

Implementing activation behavior for <label> Attempt to resolve #8179 @Manishearth , could you give me some resources having more information about what each function in `Activatable` does? The code compiles on my machine but I guess a lot more is needed  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8260)

bors-servo · 2015-10-30T13:09:35Z

💔 Test failed - mac-dev-ref-unit

Manishearth · 2015-10-30T13:24:08Z

components/script/dom/htmllabelelement.rs

+
+    // https://html.spec.whatwg.org/multipage/#implicit-submission
+    fn implicit_submission(&self, ctrlKey: bool, shiftKey: bool, altKey: bool, metaKey: bool) {
+


Add a FIXME here and file an issue for investigation

What would be the appropriate FIXME message / issue title?

File one, "Investigate and implement implicit submission for label elements"

Manishearth · 2015-10-30T13:24:17Z

Does this change any tests?

Manishearth · 2015-10-30T13:33:19Z

could you give me some resources having more information about what each function in Activatable does?

See the spec links 😄

Basically, form controls, buttons, and links can be "activated". This happens when they get clicked, or sometimes when you hit the space/enter key when they are in focus. There's a lot of complications in the spec around this, and the Activatable trait abstracts it away. Anything implementing this just needs to fill in the compulsory methods, and it's good to go. In the case of label there was no mention of canceled/post-activation steps (unlike, say, checkbox input), so it's empty.

martiansideofthemoon · 2015-10-30T13:57:32Z

@Manishearth ./mach test-ref, ./mach test-unitpassed all tests. Which test has failed? How could I see the same? Didn't find it on pressing the "Details" link

Manishearth · 2015-10-30T13:59:35Z

No, there weren't test failures, that was an induced failure (I was testing some new infrastructure). I was talking about the wpt tests.

martiansideofthemoon · 2015-10-30T14:12:08Z

What would be the correct way of checking this? I am currently running mach test-wpt and it looks like it's going to take a really long time to complete.

Manishearth · 2015-10-30T14:13:16Z

You can pass --processes 4 or something to it.

(Also, use the --log-raw parameter as described in tests/wpt/README.md so you don't need to rerun it to update expectations)

eefriedman · 2015-10-30T17:13:42Z

You probably need to change

servo/components/script/dom/element.rs

Line 1746 in 285e29c

pub fn as_maybe_activatable(&self) -> Option<&Activatable> {

before this will actually do anything.

Manishearth · 2015-10-30T19:34:34Z

Good point, forgot to check for that.

martiansideofthemoon · 2015-10-31T13:47:49Z

@Manishearth What change will be needed in that file?

Manishearth · 2015-10-31T13:48:34Z

Add a case to HTMLLabelElement to that match

Manishearth · 2015-10-31T14:26:10Z

components/script/dom/htmllabelelement.rs

+
+    // https://html.spec.whatwg.org/multipage/#implicit-submission
+    fn implicit_submission(&self, ctrlKey: bool, shiftKey: bool, altKey: bool, metaKey: bool) {
+        //FIXME: Investigate and implement implicit submission for label elements


Could you file a bug for this and refer to the bug number here?

I mean update the fixme comment here to point to that

Manishearth · 2015-10-31T14:26:34Z

@bors-servo try

Let's see if there are test issues

bors-servo · 2015-10-31T14:26:38Z

⌛ Trying commit c763010 with merge 3331e0a...

@Manishearth

Implementing activation behavior for <label> Attempt to resolve #8179 @Manishearth , could you give me some resources having more information about what each function in `Activatable` does? The code compiles on my machine but I guess a lot more is needed  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8260)

Manishearth · 2015-10-31T14:26:50Z

Small nit, then r=me unless there are test failures

bors-servo · 2015-10-31T15:03:07Z

💔 Test failed - mac-rel-wpt

Ms2ger · 2015-10-31T15:08:16Z

./components/script/dom/element.rs:1726: trailing whitespace

jdm · 2015-10-31T15:57:30Z

@bors-servo: r=Manishearth

bors-servo · 2015-10-31T15:57:31Z

📌 Commit af91c98 has been approved by Manishearth

bors-servo · 2015-10-31T16:55:29Z

⌛ Testing commit af91c98 with merge 53d8f04...

@Manishearth

Implementing activation behavior for <label> Attempt to resolve #8179 @Manishearth , could you give me some resources having more information about what each function in `Activatable` does? The code compiles on my machine but I guess a lot more is needed  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8260)

bors-servo · 2015-10-31T17:46:04Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 30, 2015

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 30, 2015

Manishearth reviewed Oct 30, 2015
View reviewed changes

martiansideofthemoon mentioned this pull request Oct 30, 2015

Investigate and implement implicit submission for label elements #8263

Closed

eefriedman removed the S-tests-failed The changes caused existing tests to fail. label Oct 30, 2015

martiansideofthemoon force-pushed the my-code-fix branch from 19e1a0d to da57e5e Compare October 31, 2015 13:44

martiansideofthemoon force-pushed the my-code-fix branch from da57e5e to c763010 Compare October 31, 2015 14:02

Manishearth reviewed Oct 31, 2015
View reviewed changes

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 31, 2015

Ms2ger added S-fails-tidy `./mach test-tidy` reported errors. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 31, 2015

Implementing activation behavior for <label>

af91c98

martiansideofthemoon force-pushed the my-code-fix branch from c763010 to af91c98 Compare October 31, 2015 15:42

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 31, 2015

bors-servo merged commit af91c98 into servo:master Oct 31, 2015

martiansideofthemoon deleted the my-code-fix branch October 31, 2015 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing activation behavior for <label> #8260

Implementing activation behavior for <label> #8260

martiansideofthemoon commented Oct 30, 2015

ManishBot commented Oct 30, 2015

bors-servo commented Oct 30, 2015

bors-servo commented Oct 30, 2015

Manishearth Oct 30, 2015

martiansideofthemoon Oct 30, 2015

Manishearth Oct 30, 2015

martiansideofthemoon Oct 30, 2015

Manishearth commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 30, 2015

Manishearth commented Oct 30, 2015

eefriedman commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 31, 2015

Manishearth commented Oct 31, 2015

Manishearth Oct 31, 2015

martiansideofthemoon Oct 31, 2015

Manishearth Oct 31, 2015

Manishearth commented Oct 31, 2015

bors-servo commented Oct 31, 2015

Manishearth commented Oct 31, 2015

bors-servo commented Oct 31, 2015

Ms2ger commented Oct 31, 2015

jdm commented Oct 31, 2015

bors-servo commented Oct 31, 2015

bors-servo commented Oct 31, 2015

bors-servo commented Oct 31, 2015


		// https://html.spec.whatwg.org/multipage/#implicit-submission
		fn implicit_submission(&self, ctrlKey: bool, shiftKey: bool, altKey: bool, metaKey: bool) {

Implementing activation behavior for <label> #8260

Implementing activation behavior for <label> #8260

Conversation

martiansideofthemoon commented Oct 30, 2015

ManishBot commented Oct 30, 2015

bors-servo commented Oct 30, 2015

bors-servo commented Oct 30, 2015

Manishearth Oct 30, 2015

Choose a reason for hiding this comment

martiansideofthemoon Oct 30, 2015

Choose a reason for hiding this comment

Manishearth Oct 30, 2015

Choose a reason for hiding this comment

martiansideofthemoon Oct 30, 2015

Choose a reason for hiding this comment

Manishearth commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 30, 2015

Manishearth commented Oct 30, 2015

eefriedman commented Oct 30, 2015

Manishearth commented Oct 30, 2015

martiansideofthemoon commented Oct 31, 2015

Manishearth commented Oct 31, 2015

Manishearth Oct 31, 2015

Choose a reason for hiding this comment

martiansideofthemoon Oct 31, 2015

Choose a reason for hiding this comment

Manishearth Oct 31, 2015

Choose a reason for hiding this comment

Manishearth commented Oct 31, 2015

bors-servo commented Oct 31, 2015

Manishearth commented Oct 31, 2015

bors-servo commented Oct 31, 2015

Ms2ger commented Oct 31, 2015

jdm commented Oct 31, 2015

bors-servo commented Oct 31, 2015

bors-servo commented Oct 31, 2015

bors-servo commented Oct 31, 2015