[print preview] Improve appearance of select control separators.

[print preview] Improve appearance of select control separators.

Separators were implemented as options with blank name/values.  These appear as
blank spaces in the select menu.  By using an HR, WebKit will render a menu
separator which looks visually nicer.

BUG=99534
TEST=Verify printer destination menu in print preview window shows platform separators.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104813

[csilv, 2011-10-07 21:53:23]

+kmadhusu for review

[kmadhusu, 2011-10-07 22:11:25]

+dpapad

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:539:
addDestinationListOption('', '', true, true, false);
dpapad: I am not sure why we have a placeholder here. If this is not a separator
we need to add aria label to this after this change.

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:621: return
document.createElement('hr');
Can you move lines 620 and 621 to a separate function because it is no longer
required in this function.

You need to call that function from lines 572, 580, 587.

[csilv, 2011-10-07 22:40:34]

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:621: return
document.createElement('hr');
On 2011/10/07 22:11:26, kmadhusu wrote:
> Can you move lines 620 and 621 to a separate function because it is no longer
> required in this function.
> 
> You need to call that function from lines 572, 580, 587.
> 

Done.

[kmadhusu, 2011-10-07 23:06:33]

lgtm

Please commit after we get a reply from dpapad@ regarding my comment.

Thanks for fixing this.

[dpapad, 2011-10-10 21:54:27]

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8207005/diff/1002/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:539:
addDestinationListOption('', '', true, true, false);
On 2011/10/07 22:11:26, kmadhusu wrote:
> dpapad: I am not sure why we have a placeholder here. If this is not a
separator
> we need to add aria label to this after this change. 

Briefly spoke with thestig about why we have this placeholder. It is needed
because otherwise when we try to get the initial print preview we will fail some
checks. So I suggest to leave it as is, and I dont think there is a need to add
aria label since the placeholder is removed and the list is populated before the
user has any chance (normally) to interact with the drop down list.