Remove activatable element filter within HTMLElement#click()

[rebstar6]

Address #6542

Ensure that click() calls are not limited to activatable elements. Also makes the isTrusted attribute false when synthetic click activation are called from a click() method (as per spec).

[jdm]

@bors-servo: try

[bors-servo]

⌛ Trying commit dc7f607 with merge 0bf06af...

[bors-servo]

💔 Test failed - mac-rel-wpt

[jdm]

  ▶ Unexpected subtest result in /DOMEvents/throwing-in-listener-when-all-have-not-run-yet.html:
  └ PASS [expected FAIL] Throwing in event listener

You'll need to remove the appropriate ini file, now that the test passes :)

[rebstar6]

Wow, that definitely was not the test that I was coding for. Seems weird that it now passes, but I can certainly take that ini out.

[jdm]

Looks good! I've only got stylistic nits about the changes, so you can go ahead and squash all the commits together when you're addressing my comments.
-S-awaiting-review +S-needs-code-changes

---

Reviewed 8 of 8 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 14 unresolved discussions.

---

components/script/dom/activation.rs, line 34 [r2] (raw file):
Let's use /// Whether an activation was initiated via the click() method. We should also add #[derive(PartialEq)] to make comparisons easier later on.

---

components/script/dom/activation.rs, line 42 [r2] (raw file):
nit: let's align these arguments with element

---

components/script/dom/activation.rs, line 46 [r2] (raw file):
Let's call this source.

---

components/script/dom/activation.rs, line 54 [r2] (raw file):
Let's add a let activatable = element.as_maybe_activatable() and use that throughout this method.

---

components/script/dom/activation.rs, line 77 [r2] (raw file):

if source == ActivationSource::FromClick {
    event.set_trusted(false);
}

---

components/script/dom/document.rs, line 8 [r2] (raw file):
use dom::activation::{ActivationSource, synthetic_click_activation};

---

components/script/dom/document.rs, line 1083 [r2] (raw file):
nit: let's move this to the previous line and align the remaining arguments with it.

---

components/script/dom/htmlbuttonelement.rs, line 7 [r2] (raw file):
use dom::activation::{Activatable, ActivationSource, synthetic_click_activation};

---

components/script/dom/htmlelement.rs, line 6 [r2] (raw file):
use dom::activation::{ActivationSource, synthetic_click_activation};

---

components/script/dom/htmlelement.rs, line 188 [r2] (raw file):
nit: ensure these arguments align with the previous line.

---

components/script/dom/htmlinputelement.rs, line 8 [r2] (raw file):
use dom::activation::{Activatable, ActivationSource, synthetic_click_activation);

---

components/script/dom/htmlinputelement.rs, line 906 [r2] (raw file):
nit: ensure these arguments are indented to match the first one.

---

components/script/dom/htmllabelelement.rs, line 7 [r2] (raw file):
use dom::activation::{Activatable, ActivationSource, synthetic_click_activation);

---

components/script/dom/htmllabelelement.rs, line 69 [r2] (raw file):
nit: indentation

---

Comments from the review on Reviewable.io

[rebstar6]

Review status: all files reviewed at latest revision, 14 unresolved discussions.

---

components/script/dom/activation.rs, line 34 [r2] (raw file):
On the enum? It's giving me: error: derive may only be applied to structs and enums

---

components/script/dom/activation.rs, line 42 [r2] (raw file):
Done.

---

components/script/dom/activation.rs, line 46 [r2] (raw file):
Done.

---

components/script/dom/activation.rs, line 54 [r2] (raw file):
Done.

---

components/script/dom/activation.rs, line 77 [r2] (raw file):
I had tried this initially but it gives errors: note: an implementation of std::cmp::PartialEq might be missing for dom::activation::ActivationSource

...which I'm now realizing is related to the partialeq comment above

---

Comments from the review on Reviewable.io

[jdm]

Yes, I meant on the enum. Apologies if that's not where I commented.

[rebstar6]

Review status: all files reviewed at latest revision, 14 unresolved discussions.

---

components/script/dom/document.rs, line 8 [r2] (raw file):
Done.

---

components/script/dom/document.rs, line 1083 [r2] (raw file):
Done.

---

components/script/dom/htmlbuttonelement.rs, line 7 [r2] (raw file):
Done.

---

components/script/dom/htmlelement.rs, line 6 [r2] (raw file):
Done.

---

components/script/dom/htmlelement.rs, line 188 [r2] (raw file):
Done.

---

components/script/dom/htmlinputelement.rs, line 8 [r2] (raw file):
Done.

---

components/script/dom/htmlinputelement.rs, line 906 [r2] (raw file):
Done.

---

components/script/dom/htmllabelelement.rs, line 7 [r2] (raw file):
Done.

---

components/script/dom/htmllabelelement.rs, line 69 [r2] (raw file):
Done.

---

Comments from the review on Reviewable.io

[rebstar6]

Comments addressed, squashed down to 1 commit.

[jdm]

One last change and this will be good to merge :)
-S-awaiting-review +S-needs-code-changes

---

Reviewed 6 of 6 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/script/dom/activation.rs, line 56 [r3] (raw file):
Let's use if let Some(a) = activatable { here (and elsewhere in this function), since it's more idiomatic than using map for side-effects.

---

Comments from the review on Reviewable.io

[rebstar6]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/script/dom/activation.rs, line 56 [r3] (raw file):
Done.

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+
Great work!

---

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit d6678a1 has been approved by jdm

[bors-servo]

⌛ Testing commit d6678a1 with merge fcceea8...

[bors-servo]

💔 Test failed - linux-rel

[jdm]

@bors-servo: retry

• #9012

[bors-servo]

⌛ Testing commit d6678a1 with merge f2f6787...

[bors-servo]

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