Closed Bug 1135364 Opened 9 years ago Closed 9 years ago

close <meta> tag in aboutReader.html

Categories

(Toolkit :: Reader Mode, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: abdelrahman, Assigned: bkudryavtsev, Mentored)

Details

(Whiteboard: [good first bug])

Attachments

(2 files, 2 obsolete files)

Whiteboard: [good first bug]
Attachment #8567588 - Flags: review?(bnicholson)
Hi, I'm new to open source and trying to get involved and this bug looked like an easy starting point. I have fixed it on my local build and would like to know how I can submit my patch for review. It would also be great if I could have this bug assigned to me. (I'm in an open source class and would like to have a record of what I've worked on.) Thanks in advance for the help!
Attached patch aboutReader.html (obsolete) — Splinter Review
Is this how I'm supposed to submit a patch? Sorry. Again, I'm new to all of this.
Attachment #8567767 - Flags: review?(a.ahmed1026)
Never mind, I didn't realize the post above mine was also a patch request until I had already posted. Sorry, like I said, I'm new to all of this.
(In reply to Kyle Thomas from comment #3)
> Created attachment 8567767 [details] [diff] [review]
> aboutReader.html
> 
> Is this how I'm supposed to submit a patch? Sorry. Again, I'm new to all of
> this.

Hi Kyle,
You can start with developer guide (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide)
and also there are some tutorials on codefirefox (http://codefirefox.com)
becasue this patch does not seem to be correct and feel free to ask me if
you have a problem with it.

(In reply to Kyle Thomas from comment #4)
> Never mind, I didn't realize the post above mine was also a patch request
> until I had already posted. Sorry, like I said, I'm new to all of this.

Sorry, this bug will be assigned to Boris because his patch seems to be ok
and he submitted it before you.
Attachment #8567767 - Flags: review?(a.ahmed1026)
Assignee: nobody → boriskk.work
Status: NEW → ASSIGNED
Attachment #8567767 - Attachment is obsolete: true
Comment on attachment 8567588 [details] [diff] [review]
rev1 - Close <meta> tag

Review of attachment 8567588 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Thanks!
Attachment #8567588 - Flags: review?(bnicholson) → review+
There really is no reason to do this, it's just wasted bytes.
No reason in terms of correctness, sure, but it's a small bit of cleanup to keep the two <meta> lines consistent (one was closed, one was not).
Drive-by: I believe we could also shorten this to: <meta charset="utf-8" />

https://hg.mozilla.org/integration/fx-team/rev/26881868e390
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/26881868e390
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][fixed-in-fx-team] → [good first bug]
Target Milestone: --- → mozilla39
Attached patch Patch for 38 (obsolete) — Splinter Review
Approval Request Comment
[Feature/regressing bug #]: reading list/reader mode
[User impact if declined]: 38 will have partial reading list/reader mode support
[Describe test coverage new/current, TreeHerder]: automated tests and manual tests
[Risks and why]: none expected
[String/UUID change made/needed]: none
Attachment #8583398 - Flags: approval-mozilla-aurora?
Attached patch Patch for 38Splinter Review
Approval Request Comment
[Feature/regressing bug #]: reading list/reader mode
[User impact if declined]: 38 will have partial reading list/reader mode support
[Describe test coverage new/current, TreeHerder]: automated tests and manual tests
[Risks and why]: none expected
[String/UUID change made/needed]: none
Attachment #8583398 - Attachment is obsolete: true
Attachment #8583398 - Flags: approval-mozilla-aurora?
Attachment #8583400 - Flags: approval-mozilla-aurora?
Comment on attachment 8583400 [details] [diff] [review]
Patch for 38

It looks like reading list/reader mode has broad approval and I can just use a=readinglist

https://hg.mozilla.org/releases/mozilla-aurora/rev/fa8645257f11 for reference
Attachment #8583400 - Flags: approval-mozilla-aurora?
Boris, is manual QA verification required here? If so, could you please expand a bit on how should we proceed?
Flags: qe-verify?
Flags: needinfo?(boriskk.work)
No need.
Flags: qe-verify?
Flags: qe-verify-
Flags: needinfo?(boriskk.work)
You need to log in before you can comment on or make changes to this bug.