GlobalEventHandlers have different docs URL on mdn then attribute

[lalitsom]

getGlobalAtributeDocsURL function in lib/provider.coffee was returning
https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/{attributename} for all attributes,
but Events like onlick, onselect have different URL on mdn
https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/{eventname}.

That's why I filtered the return value of this function depending on attribute name,
if attribute starts with "on" which means an event
then doc URL will be https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/{eventname}
else
same as it were before.

[winstliu]

Thanks. Can you add a test for this please to make sure it doesn't regress?

[lalitsom]

Hi,
this is my first time working with atom packages, so I don't know what kind of test you are talking about, should I add an image or gif which shows that it's working.

ok, I think you mean add test assertions in specs, never done that before but I'll try.

[winstliu]

@lalitsom what do you think about the following change instead (based off of master)?

diff --git a/lib/provider.coffee b/lib/provider.coffee
index ff4ca9d..edb509a 100644
--- a/lib/provider.coffee
+++ b/lib/provider.coffee
@@ -118,7 +118,7 @@ module.exports =
     displayText: attribute
     type: 'attribute'
     description: description ? "Global #{attribute} attribute"
-    descriptionMoreURL: if description then @getGlobalAttributeDocsURL(attribute) else null
+    descriptionMoreURL: @getGlobalAttributeDocsURL(attribute)

   getAttributeValueCompletions: ({prefix, editor, bufferPosition}) ->
     completions = []
@@ -181,7 +181,13 @@ module.exports =
     "#{@getTagDocsURL(tag)}#attr-#{attribute}"

   getGlobalAttributeDocsURL: (attribute) ->
-    "https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/#{attribute}"
+    if attribute.startsWith('on')
+      "https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/#{attribute}"
+    else if attribute.startsWith('aria-')
+      # As of September 2017, MDN does not have pages for ARIA attributes
+      "https://www.w3.org/TR/wai-aria-1.1/##{attribute}"
+    else
+      "https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/#{attribute}"

 firstCharsEqual = (str1, str2) ->
   str1[0].toLowerCase() is str2[0].toLowerCase()

That should cover all global attributes.

[lalitsom]

yup, this one looks better.
I think it will cover almost all attributes, except for "role".
"role" also have the same base URL as aria-attribute, so we can use that.
https://www.w3.org/TR/wai-aria-1.1/#usage_intro, but it uses #usage_intro instead of #role.

@50Wliu can you tell me how can I run the tests from spec.coffee on my local computer.

[winstliu]

Sounds good. You can run specs from Atom: search the command palette for window:run-package-specs. Or, if you prefer the command line, the command is atom --test spec. However, in both cases make sure that you have run apm install beforehand (not strictly necessary for this package, but for most others it is).

[winstliu]

attribute is 'role'

[winstliu]

description is not used and can be removed

[winstliu]

description is not used and can be removed

[winstliu]

"provide" -> "provides"

Similar tests can also be added now for aria- and role.

[lalitsom]

ok, thanks for the review.
I have added the test cases for "role", but I'm unable to add the test for "aria-" attribute.
Maybe there is a bug, at getAttributeNameCompletions in provider.cofeee file.

autocomplete-html/lib/provider.coffee

Lines 95 to 106 in 4049b13

	getAttributeNameCompletions: ({prefix, editor, bufferPosition}) ->
	completions = []
	tag = @getPreviousTag(editor, bufferPosition)
	tagAttributes = @getTagAttributes(tag)
	for attribute in tagAttributes when not prefix.trim() or firstCharsEqual(attribute, prefix)
	completions.push(@buildLocalAttributeCompletion(attribute, tag, @completions.attributes[attribute]))
	for attribute, options of @completions.attributes when not prefix.trim() or firstCharsEqual(attribute, prefix)
	completions.push(@buildGlobalAttributeCompletion(attribute, options)) if options.global
	completions

It returns an array of attributes whose first element matches with prefix,
for prefix 'aria-' it returns accesskey as first element. I think instead of checking the firstCharsEqual it should check if attribute contains prefix.
Don't know why but at testing manually it gives correct suggestions that is aria-busy at first element instead of accesskey.

[winstliu]

What's happening is that we provide the raw completions list to autocomplete-plus, which then does additional filtering. However, the specs are testing the raw completions and not the filtered list. This spec should work:

it "provides a descriptionMoreURL if the attribute does not have a unique description but starts with aria-*", ->
    editor.setText('<div aria-b')
    editor.setCursorBufferPosition([0, 11])

    completions = getCompletions()
    console.log completions

    expect(completions[0].displayText).toBe 'aria-busy'
    expect(completions[0].description).toBe 'Global aria-busy attribute'
    expect(completions[0].descriptionMoreURL.endsWith('/TR/wai-aria-1.1/#aria-busy')).toBe true

[winstliu]

Nevermind, even that doesn't work. I'll have to get back to you on how best to solve this.

[lalitsom]

What if we filter the raw suggestions in spec file, according to prefix value.
suggestions returned by provider are already in sorted form, we can just remove those attributes that does not contain prefix as a substring in it.

As of now filterByPrefix function will filter the list, only if prefix is of atleast length 3, so that test cases like "autcompletes tag names without a prefix" would still work.

I also noticed that autocomplete-plus does not have the right order for suggestions ( check screenshot in my last comment), 'aria-atomic' and 'aria-activedescendant' should be above than 'aria-busy''.
Maybe it is also considering the attribute length as a sorting paramater (small length attribute will appear first).

[lalitsom]

@50Wliu , can you give me a review on my last commit. Not sure, if this should be the way to solve this problem.
I think the issue of docsURL is solved so maybe we should create a new issue for this problem.
Any suggestions would be helpful.

[winstliu]

Will look at it soon.

[Alfian878787]

Duplicate of #

[lalitsom]

Duplicate of what?