Closed Bug 1412922 Opened 5 years ago Closed 5 years ago

dom/tests/mochitest/bugs/test_DOMWindowCreated_chromeonly.html lacks script end tag

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: hsivonen, Assigned: ravipaul12321)

Details

(Keywords: good-first-bug)

Attachments

(2 files)

dom/tests/mochitest/bugs/test_DOMWindowCreated_chromeonly.html  lacks </script>
Keywords: good-first-bug
Priority: -- → P3
Hi, I would like to take this on as my first bug.

I will submit a patch for this shortly.
I've attached the patch. Please help me with testing it. Thank you, Regards
Attachment #8931315 - Flags: review?(overholt)
Thanks for the patch! It looks good to me. Are you able to run it through try (see [1])?

[1]
https://wiki.mozilla.org/ReleaseEngineering/TryServer#Getting_access_to_the_Try_Server
Assignee: nobody → ravipaul12321
Flags: needinfo?(ravipaul12321)
Comment on attachment 8931315 [details] [diff] [review]
Patch for Bug 1412922- Adding a closing Script tag

Blake's in a better position to review this than I am (and I'm not a peer).
Attachment #8931315 - Flags: review?(overholt) → review?(mrbkap)
Okay, thanks Andrew.

I'm trying to make a patch for the TryServer using 'mq' but i'm having trouble understanding the Try syntax. Would I run a test with the default configurations?
(In reply to ravipaul12321 from comment #5)
> Okay, thanks Andrew.
> 
> I'm trying to make a patch for the TryServer using 'mq' but i'm having
> trouble understanding the Try syntax. Would I run a test with the default
> configurations?

Yes, you can just run with the default syntax. You can use https://mozilla-releng.net/trychooser/ if you need help with it.
Flags: needinfo?(ravipaul12321)
I've created a valid 'try' patch and have attempted to push this to the TryServer, however, I have encountered an issue where i'm given a 'permission denied (publickey).

I have taken this bug on as a part of my university course and have been advised that I may require an SSH key to resolve the issue, and I may not have time to get a SSH key approved to have access to the TryServer.

If it is possible, could another reviewer please test this code for me and determine whether the test completes successfully and this is a viable patch?

Thanks again.
Comment on attachment 8931315 [details] [diff] [review]
Patch for Bug 1412922- Adding a closing Script tag

Thanks for the patch. I've tested this locally and it looks good. While you're here, though, would you mind converting this file to Unix-style line endings (LF only)? A tool like dos2unix or most editors can do that for you.

Once you do that, you'll want to attach a patch that someone will be able to push for you. You can do that with the bzexport (and using 'hg bzexport') or by committing your changes to a mercurial changeset, doing 'hg export . >patch' and uploading the resulting "patch" file.

The commit message for Firefox changes has a particular format you can read about at [1].

Let me know if you have any questions or problems.

[1] https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I...
Attachment #8931315 - Flags: review?(mrbkap) → review+
I've added the UNIX-Style line endings to the file and ran the patch through hg export. Could you let me know if this is okay?
Many Thanks.
Attachment #8934971 - Flags: review?(mrbkap)
Comment on attachment 8934971 [details] [diff] [review]
Bug-1412922.patch

This looks great! Thanks.
Attachment #8934971 - Flags: review?(mrbkap) → review+
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cce48b49d3d
Added closing script tag and converted file to UNIX-style line endings. r=mrbkap
https://hg.mozilla.org/mozilla-central/rev/9cce48b49d3d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.