379011 - Text size increasing/decreasing broken in view source window

Status: VERIFIED FIXED

(Toolkit :: View Source, defect)

Description

[Gerald]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070426 GranParadiso/3.0a4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070426 GranParadiso/3.0a4pre

I used the pre alpha 4 version on windows xp. I wanted to increase the text size in the view source page but it does not work on windows xp with alpha 4. Also Ctrl-- and Ctrl-+ do nothing. 

Reproducible: Always

Steps to Reproduce:
1. Load a website
2. click View Page source in the context menu
3. View->Text Size->Increase or Decrease does nothing


Expected Results:  
Increase/decrease the text size.

Comment 1

[[:Aleksej]]

Confirmed (including Ctrl-+ and Ctrl--) on:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a4pre) Gecko/20070426 GranParadiso/3.0a4pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070206 Minefield/3.0a2pre
Thunderbird version 3.0a1 (20070427) Linux

will search for the regression date

Please, change the OS field to "All"

Comment 2

[[:Aleksej]]

3.0a1-trunk.linux-i686:
tr2006091904 - wfm
tr2006092004 - cfm

Comment 3

[[:Aleksej]]

(The above comment is about Minefield.)

Comment 4

[Martijn Wargers (dead)]

I'm also seeing this bug in current trunk build of Firefox.
Bonsai link of regression range of comment 3:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-09-19+04&maxdate=2006-09-20+04&cvsroot=%2Fcvsroot
It seems that the patch from bug 286365 has caused this.

The getMarkupDocumentViewer() function from viewSource.js has been removed there. I suspect it is still necessary to get the text size increasing/decreasing to work in view source.

Comment 5

[Mike Connor [:mconnor]]

Jason, do you have cycles to fix this?  If not, Gavin, can you take this?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Jason, please steal this away from me if you want, I can review.

Comment 7

[Jason Barnabe (np)]

Yoink.

Comment 8

[Jason Barnabe (np)]

Attachment: add getMarkupDocumentViewer back

getMarkupDocumentViewer was removed in bug 286365 because its only caller in the file was removed. Turns out it's required by viewZoomOverlay.js. This adds it back and adds a comment explaining why it's there.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Can you just add it to viewZoomOverlay instead, and use getBrowser() like the other callers in that file? These files really shouldn't depend on /browser like they do... :(

Comment 10

[Jason Barnabe (np)]

Attachment: move getMarkupDocumentViewer to viewZoomOverlay.js

OK, this adds getMarkupDocumentViewer to viewZoomOverlay.js, updates the comment there to say that you need to have getBrowser, and removes getMarkupDocumentViewer from help.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 267518 [details] [diff] [review]
move getMarkupDocumentViewer to viewZoomOverlay.js

This no longer really needs to be a function anymore, now that it's on an object (could just be a property of the object). r=me with that change, if you want to make it.

Comment 12

[Jason Barnabe (np)]

Wouldn't that require that viewZoomOverlay.js be loaded after whatever defines getBrowser(), and then getBrowser() would have to work before the document's loaded?

I'd also prefer to keep it as it is to slightly reduce code repetition.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Good point - could still be a getter, though. Not sure what you mean by "avoid code repetition", a property/getter would be no more repetitive than the current code.

Comment 14

[Jason Barnabe (np)]

Regarding repetition, I was comparing to having getBrowser().markupDocumentViewer in the two places where this.getMarkupDocumentViewer() is now.

I can switch it to a getter.

Comment 15

[Jason Barnabe (np)]

Attachment: use a getter

Comment 16

[Phil Ringnalda (:philor)]

toolkit/components/help/content/help.js 1.45
toolkit/content/viewZoomOverlay.js 1.7

Comment 17

[Jeff Walden [:Waldo]]

Suggest Litmus ftw.

Comment 18

[[:Aleksej]]

Verified
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070615 Minefield/3.0a6pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007071804 Minefield/3.0a7pre

Comment 19

[juan becerra [:juanb]]

Marcia, Martijn, this may fall under some of your Fx3 areas. Could you add a test case in litmus to cover this? This is similar to increasing/decreasing the font on a web page.

Comment 20

[Martijn Wargers (dead)]

Ok, I added http://litmus.mozilla.org/show_test.cgi?id=4703

Comment 21

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010805 Minefield/3.0b3pre ID:2008010805