753108 - javascript alerts can't be zoomed

Status: NEW

(Toolkit :: Content Prompts, defect, P5)

Description

[Dan Jacobson]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20120504 Firefox/14.0a2 Iceweasel/14.0a2
Build ID: 20120504042005

Steps to reproduce:

had an alert pop up in front of me


Actual results:

tried to zoom it with CTRL+++++


Expected results:

The page behind it got zoomed, not the alert!

How are older users supposed to read alerts if they show up too small.

Comment 1

[Dan Jacobson]

Who in the world wants to zoom the greyed out background page when we are staring at an alert?!?!

Comment 2

[Dan Jacobson]

https://ipost.post.gov.tw/web/CSController
function setDef() {
	 //if (typeof(document.MainForm) == 'object')
	 		//focusObject(document.MainForm.USERID);
  try{
  	if(document._HSM_App==null){
  		alert("目前環境設定為未開啟HSM，無法測試JRE是否安裝");
  	}	else if(document._HSM_App.isActive()){
			//alert("jre已安裝");
		}
  }catch(error){
    var jreDownloadUrl = "http://java.com/zh_TW/";
    var alertMsg = "一、系統檢測您的電腦尚未安裝Java程式，或未正確安裝及設定，致無法啟動運作。\r\n\r\n" +
                   "二、若您尚未安裝Java程式，請點選「確定」鍵，連結至Java官網，依網頁指示下載安裝。\r\n" +
                   "　　安裝完成後，請關閉瀏覽器（不用重開機），再重新登入網路郵局。\r\n\r\n" +
                   "三、如您已安裝Java程式仍無法登入網路郵局，請提供問題畫面（按鍵盤右上角\"Prt Scr\"鍵\r\n" +
                   "　　再至信件或Word檔貼上畫面），Email至網路郵局信箱（首頁左下方「聯絡我們」），\r\n" +
                   "　　我們將儘速協助您解決。";
      if(confirm(alertMsg)){
        top.location.href=jreDownloadUrl;
      }  	
  }     
}

Comment 3

[Drew Willcoxon :adw]

Maybe zoom should affect JS alerts, especially now that they're tab modal and in content.  Dolske?

Comment 4

[Justin Dolske [:Dolske]]

I'm ambivalent on the matter... Alerts are technically chrome, so page-zoom shouldn't affect them. But there's also kind of intended to look like content, so maybe it should. Neither approach is obviously right or wrong, imo. I'd take a patch if it's simple, but if it's complex I'd rather not.

Comment 5

[Vignesh Shanmugam]

Hi, Can I take this bug and work on it?. This is going to be my first bug that I am going to contribute to Firefox.

Comment 6

[Dan Jacobson]

(In reply to Vignesh Shanmugam from comment #5)
I think everybody would be happy if someone (e.g., you) would fix the bug.

Comment 7

[Vignesh Shanmugam]

Can you tell me how to get the code base to start working on this?.

Comment 8

[Dan Jacobson]

(In reply to Vignesh Shanmugam from comment #7)
I don't know. I'm just the reporter.

Comment 9

[Drew Willcoxon :adw]

Hi Vignesh, thanks for volunteering.  You can find information on patching Firefox starting here: https://developer.mozilla.org/en-US/docs/Developer_Guide

It would probably be easiest to intercept the zoom commands and resize the prompt instead of the page if a prompt is showing.  The zoom code path looks like this:

When you change the zoom, the cmd_fullZoomReduce and related commands are called.  They're defined here:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-sets.inc#82

The commands call the FullZoom object here:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullZoom.js

FullZoom calls the ZoomManager object here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/viewZoomOverlay.js

Finally, ZoomManager sets the browser's textZoom and fullZoom properties, which are here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/browser.xml#430

You could do the intercept in the textZoom and fullZoom setters.

I'm not sure how you could tell whether a prompt is showing.  It looks like browsers have a "tabmodalPromptShowing" attribute, so it seems like that would work.

Post back here if you have questions.  You can also visit the front-end development chat room at #fx-team on irc.mozilla.org.  I'm adw there.

Comment 10

[Vignesh Shanmugam]

Hi Drew, Thanks for assigining to me. Will go through the code and come up with fix soon :)(In reply to Drew Willcoxon :adw from comment #9)
> Hi Vignesh, thanks for volunteering.  You can find information on patching
> Firefox starting here:
> https://developer.mozilla.org/en-US/docs/Developer_Guide
> 
> It would probably be easiest to intercept the zoom commands and resize the
> prompt instead of the page if a prompt is showing.  The zoom code path looks
> like this:
> 
> When you change the zoom, the cmd_fullZoomReduce and related commands are
> called.  They're defined here:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-
> sets.inc#82
> 
> The commands call the FullZoom object here:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-
> fullZoom.js
> 
> FullZoom calls the ZoomManager object here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/
> viewZoomOverlay.js
> 
> Finally, ZoomManager sets the browser's textZoom and fullZoom properties,
> which are here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/
> browser.xml#430
> 
> You could do the intercept in the textZoom and fullZoom setters.
> 
> I'm not sure how you could tell whether a prompt is showing.  It looks like
> browsers have a "tabmodalPromptShowing" attribute, so it seems like that
> would work.
> 
> Post back here if you have questions.  You can also visit the front-end
> development chat room at #fx-team on irc.mozilla.org.  I'm adw there.

Comment 11

[Vignesh Shanmugam]

Done with the code. Trying to write the browser tests. Can you help me on writing the test cases for this bug?

Comment 12

[Drew Willcoxon :adw]

I mentioned before that you'd need to write a browser chrome test, but it looks like the tab-modal prompt tests are plain Mochitests that use the SpecialPowers API to access chrome.  So you can probably write a plain Mochitest, too.  Mochitests are described here: https://developer.mozilla.org/en-US/docs/Mochitest

Your test will need to ensure that zooming affects the tab-modal prompt.

You can see the existing tab-modal prompt tests in this directory: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/prompts/test/  The tests are the files whose names start with "test_".  You should read them to see how they work.  Most of them include the prompt_common.js file also in that directory, and you'll probably find that file helpful, too.  Your test should live in the same directory.  You might start by copying one of the existing tests.  And don't forget to add your new file to the mochitest.ini file in that directory.

Could you please attach your patch -- what you have so far -- to this bug?

Comment 13

[Drew Willcoxon :adw]

I forgot about mouse wheel zooming.  We should handle that, too.  It's implemented deep within Gecko: http://mxr.mozilla.org/mozilla-central/source/dom/events/nsEventStateManager.cpp#3470

There's a "DOMMouseScroll" event you can listen for.  Looking at the code I linked to, I can't tell whether it's possible to cancel that event to prevent the zoom.  If not, then we can either try to modify that code so it is possible, or we should just not try to prevent the page from zooming when you zoom a prompt.  I think it's OK to zoom both.

Either way, I think you'll want to listen for DOMMouseScroll, and if the scroll is a zoom event, then zoom the prompt accordingly.  You can see how FullZoom listens for zoom DOMMouseScroll events: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullZoom.js#85

Comment 14

[Matthew N. [:MattN]]

Bulk move to Toolkit::Notifications and Alerts

Filter on notifications-and-alerts-component.

Comment 15

[Dão Gottwald [:dao]]

I think this is invalid/wontfix. An alert's text size is independent from the page behind it, so they shouldn't share a zoom value.

(In reply to Dan Jacobson from comment #0)
> How are older users supposed to read alerts if they show up too small.

The text size used by alerts depends on your operating system's DPI and text size settings, just like the text in the location bar or tab labels, for instance. If you can't read alerts, you won't be able to read other UI text in Firefox and other applications either. To fix that, you should adjust your operating system's settings.

Comment 16

[Dão Gottwald [:dao]]

We could however ignore zoom commands while a tab-modal dialog is present, since this is a valid question:

(In reply to Dan Jacobson from comment #1)
> Who in the world wants to zoom the greyed out background page when we are
> staring at an alert?!?!

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:tspurway, could you have a look please?
For more information, please visit auto_nag documentation.