Closed
Bug 703965
Opened 13 years ago
Closed 13 years ago
View source prefs not obeyed in non-HTML documents
Categories
(Toolkit :: View Source, defect)
Toolkit
View Source
Tracking
()
RESOLVED
FIXED
mozilla11
Tracking | Status | |
---|---|---|
firefox10 | --- | unaffected |
People
(Reporter: darktrojan, Assigned: hsivonen)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
smaug
:
review+
christian
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
christian
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
Landing the HTML5 parser for view source has broken view_source.tab_size and view_source.wrap_long_lines for documents that don't use nsHTML5Highlighter, ie. non-HTML documents such as javascript.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #576950 -
Attachment is patch: true
Reporter | ||
Comment 2•13 years ago
|
||
Here's a test, it's just copy of the other prefs test with a non-html document.
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Geoff Lankow (:darktrojan) from comment #2)
> Created attachment 577009 [details] [diff] [review] [diff] [details] [review]
> tests
>
> Here's a test, it's just copy of the other prefs test with a non-html
> document.
Thanks, but it looks like you forgot to hg add browser_viewsourceprefs_nonhtml.js.
Assignee | ||
Comment 4•13 years ago
|
||
Any ideas why the test fails on try but passes locally when run individually?
https://tbpl.mozilla.org/php/getParsedLog.php?id=7608812&tree=Try
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #576950 -
Attachment is obsolete: true
Reporter | ||
Comment 6•13 years ago
|
||
There's something about pushPrefEnv I don't fully understand, so I threw it out and used a cleanup function instead. Seems to do the trick. https://tbpl.mozilla.org/?tree=Try&rev=ffc26efcbc25
Attachment #577009 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #577787 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Attachment #577577 -
Flags: review?(bugs)
Comment 7•13 years ago
|
||
Comment on attachment 577577 [details] [diff] [review]
Code changes without test changes
>+ * The Initial Developer of the Original Code is
>+ * Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2010
2010 or 2011?
>+ * The Initial Developer of the Original Code is
>+ * Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2010
2010 or 2011?
Attachment #577577 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #7)
> 2010 or 2011?
The .cpp code was copied and pasted from a file that said 2010, so 2010. I guess the .h file should be 2011, though. I'll land with the .h as 2011.
Thanks.
Assignee | ||
Comment 9•13 years ago
|
||
Comment 10•13 years ago
|
||
This seems to be the cause of the M-Oth orange here:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c5d44e6e957b
Comment 11•13 years ago
|
||
Would appear the changeset that landed was missing a few chunks compared to that sent to try:
https://hg.mozilla.org/try/rev/9856bc882913
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe8a47afa807
Comment 12•13 years ago
|
||
Backed out on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20a24dd3f56b
Reporter | ||
Comment 13•13 years ago
|
||
Assignee | ||
Comment 14•13 years ago
|
||
Sorry about landing the wrong patch for the tests. Thanks for relanding.
Comment 15•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b6d8f601d865
https://hg.mozilla.org/mozilla-central/rev/81a01033b52d
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Assignee | ||
Updated•13 years ago
|
status-firefox10:
--- → affected
Assignee | ||
Comment 16•13 years ago
|
||
Comment on attachment 577577 [details] [diff] [review]
Code changes without test changes
(This comment has been posted on all the bugs mentioned in this comment, except bug 710142, so that the release drivers see it regardless of the order in which they process approval requests.)
The new View Source implementation landed before Firefox 10 moved to Aurora. Afterwards, a bunch of regressions were identified. Many of the regression fixes didn't land before Firefox 10 moved to Aurora but they have now been fixed on trunk except for bug 710142.
To avoid shipping with regressions, we either need to land all the regression fixes on Aurora for Firefox 10 (followed by a fix for bug 710142 in Beta if it doesn't make it before Dec 20th) or switch back to the old View Source implementation on Aurora. The new View Source implementation provides much better diagnostics for Web developers than the old View Source implementation.
So I'd like to ask the release drivers to either approve bug 535530, bug 699356, bug 699365, bug 700034, bug 700361, bug 703965, bug 704667 and bug 705473 plus bug 695640, which is a non-regression tweak, or to approve bug 710175 for reverting to the old View Source implementation for Firefox 10.
Attachment #577577 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•13 years ago
|
Attachment #577787 -
Flags: approval-mozilla-aurora?
Comment 17•13 years ago
|
||
Comment on attachment 577577 [details] [diff] [review]
Code changes without test changes
[triage comment]
We decided to back out the new view source parser (bug 710175) rather than take this fixup for Firefox 10.
Denying for Aurora.
Attachment #577577 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Attachment #577787 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Assignee | ||
Comment 18•13 years ago
|
||
Firefox 10 no longer affected due to bug 710175 landing.
You need to log in
before you can comment on or make changes to this bug.
Description
•