44 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
Check 185: id attribute is not unique. Repair: Modify the id attribute value so it is unique. Error Line 449, Column 3: <body id="home" class="html-ltr lang-en-US"> <div id="strings" data-global-close="Close" ...(download-button-desktop-release) see the suggestions for it at http://achecker.ca/checker/suggestion.php?id=185 PS I used http://achecker.ca/checker/index.php and put in mozilla.org as the domain name
The download button is embedded twice on the home page: once in the promo grid and again in the gray section below the promo grid. The button macro includes an ID so when that macro repeats on a page it will repeat the ID as well. https://github.com/mozilla/bedrock/blob/master/bedrock/firefox/templates/firefox/includes/download-button.html#L25 If we don't actually need that ID for anything we could simply remove it.
Component: General → Pages & Content
Summary: Compatible: Maximize compatibility with current and future user agents, including assistive technologies → Duplicate ID attributes on home page
The id value here always seems to be "download-button-desktop-release" from what I can tell, even when spoofing a mobile device such as Android or iOS using Chrome. The only JS that actually uses this id is a couple of functional tests, which can just as safely test for .download-button so, we can more than likely just go ahead and drop the id altogether. With that said, what are your thoughts Alex?
The functional tests only look for the class, so no issue there I think: https://github.com/mozilla/bedrock/blob/master/tests/functional/home.js#L18 As Craig says, this is just the default ID for the download button passed via the template macro. We can simply change it so they differ, or remove the ID. Should make no difference.
Thanks for the feedback Alex. Some of the other functional tests do look for the id. I will fix that along with dropping the id from the element.
Assignee: nobody → schalk.neethling.bugs
Created attachment 8651761 [details] [review] Link to Github pull-request: https://github.com/mozilla/bedrock/pull/3237
(In reply to Schalk Neethling [:espressive] from comment #4) > Thanks for the feedback Alex. Some of the other functional tests do look for > the id. I will fix that along with dropping the id from the element. Oh, I had assumed fixing this bug would simply be removing the duplicate IDs from the home page, and not removing the property from the download button altogether?
I guess if we have no use for the ID dropping it from the download button macro might still be worthwhile? I can't see any references to it in either CSS or JS, but we should be sure to double-check.
I could not find any use of it anywhere other than in some of the functional tests.
Commits pushed to master at https://github.com/mozilla/bedrock https://github.com/mozilla/bedrock/commit/6bafbfec2e302d423557731aa77b545c720f07ee Fix Bug 1196471, remove id from download button to avoid duplicate ids https://github.com/mozilla/bedrock/commit/62dabd6bd9a9f5d5e6e5df9e6bedbe757d71e3f8 Merge pull request #3237 from schalkneethling/bug1196471-duplicate-ids-on-home-page Fix Bug 1196471, remove id from download button to avoid duplicate ids
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.