Modifes the in-application EULA outer frame to take a parameter ('innerframe'...

Modifes the in-application EULA outer frame to take a parameter ('innerframe') and set its inner iframe src to the value of that parameter.

BUG=8894
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12415

[Glenn Wilson, 2009-03-19 22:40:10]

Includes a change to the javascript to look for the 'innerframe' as the
parameter name and sets the src of the iframe to the value of this param.

[Mark Larson, 2009-03-20 01:17:45]

I think you can avoid the regexp with code that gets the value explicitly (not
relying on your regex kung-fu to match):

var params = window.location.search.substring(1).split('&');
for i in params {
  key_value_pair = params[i].split('=');
  if (key_value_pair.length == 2 && 'innerframe' == key_value_pair[0]) {
    document.getElementById('ifr').src = key_value_pair[1];
    return;
  }
}

I really recommend getting someone with better JS knowledge than me and CPU to
weigh in.

[cpu_(ooo_6.6-7.5), 2009-03-20 01:43:31]

According to a javascript programmer  the two options are .split or regexp, I am
fine with whatever.

thus LGTM


On 2009/03/20 01:17:45, Mark Larson wrote:
> I think you can avoid the regexp with code that gets the value explicitly (not
> relying on your regex kung-fu to match):
> 
> var params = window.location.search.substring(1).split('&');
> for i in params {
>   key_value_pair = params[i].split('=');
>   if (key_value_pair.length == 2 && 'innerframe' == key_value_pair[0]) {
>     document.getElementById('ifr').src = key_value_pair[1];
>     return;
>   }
> }
> 
> I really recommend getting someone with better JS knowledge than me and CPU to
> weigh in.

[cpu_(ooo_6.6-7.5), 2009-03-21 03:09:22]

By the way, what happens if the param is in quotes?

eula?innerframe="a path"

and

eula?innerframe=apath

should work

On 2009/03/20 01:43:31, cpu wrote:
> According to a javascript programmer  the two options are .split or regexp, I
am
> fine with whatever.
> 
> thus LGTM
> 
> 
> On 2009/03/20 01:17:45, Mark Larson wrote:
> > I think you can avoid the regexp with code that gets the value explicitly
(not
> > relying on your regex kung-fu to match):
> > 
> > var params = window.location.search.substring(1).split('&');
> > for i in params {
> >   key_value_pair = params[i].split('=');
> >   if (key_value_pair.length == 2 && 'innerframe' == key_value_pair[0]) {
> >     document.getElementById('ifr').src = key_value_pair[1];
> >     return;
> >   }
> > }
> > 
> > I really recommend getting someone with better JS knowledge than me and CPU
to
> > weigh in.

[cpu_(ooo_6.6-7.5), 2009-03-24 01:17:24]

Ping mal. Ping gwilson.

On 2009/03/21 03:09:22, cpu wrote:
> By the way, what happens if the param is in quotes?
> 
> eula?innerframe="a path"
> 
> and
> 
> eula?innerframe=apath
> 
> should work
> 
> On 2009/03/20 01:43:31, cpu wrote:
> > According to a javascript programmer  the two options are .split or regexp,
I
> am
> > fine with whatever.
> > 
> > thus LGTM
> > 
> > 
> > On 2009/03/20 01:17:45, Mark Larson wrote:
> > > I think you can avoid the regexp with code that gets the value explicitly
> (not
> > > relying on your regex kung-fu to match):
> > > 
> > > var params = window.location.search.substring(1).split('&');
> > > for i in params {
> > >   key_value_pair = params[i].split('=');
> > >   if (key_value_pair.length == 2 && 'innerframe' == key_value_pair[0]) {
> > >     document.getElementById('ifr').src = key_value_pair[1];
> > >     return;
> > >   }
> > > }
> > > 
> > > I really recommend getting someone with better JS knowledge than me and
CPU
> to
> > > weigh in.

[Glenn Wilson, 2009-03-24 01:20:45]

My earlier response did not get forwarded here.  Strange.

Well, technically, a URL parameter can't have spaces or quotes in it...they must
be URL-encoded like 

eula?innerframe=%22a%20path%22

However, I can also add support for spaces and quotes if it simplifies things. 
I'd probably rather use a regex in this case because it makes the split much
simpler

I have this modified locally...testing the change now.

On 2009/03/24 01:17:24, cpu wrote:
> Ping mal. Ping gwilson.
> 
> On 2009/03/21 03:09:22, cpu wrote:
> > By the way, what happens if the param is in quotes?
> > 
> > eula?innerframe="a path"
> > 
> > and
> > 
> > eula?innerframe=apath
> > 
> > should work
> > 
> > On 2009/03/20 01:43:31, cpu wrote:
> > > According to a javascript programmer  the two options are .split or
regexp,
> I
> > am
> > > fine with whatever.
> > > 
> > > thus LGTM
> > > 
> > > 
> > > On 2009/03/20 01:17:45, Mark Larson wrote:
> > > > I think you can avoid the regexp with code that gets the value
explicitly
> > (not
> > > > relying on your regex kung-fu to match):
> > > > 
> > > > var params = window.location.search.substring(1).split('&');
> > > > for i in params {
> > > >   key_value_pair = params[i].split('=');
> > > >   if (key_value_pair.length == 2 && 'innerframe' == key_value_pair[0]) {
> > > >     document.getElementById('ifr').src = key_value_pair[1];
> > > >     return;
> > > >   }
> > > > }
> > > > 
> > > > I really recommend getting someone with better JS knowledge than me and
> CPU
> > to
> > > > weigh in.

[Glenn Wilson, 2009-03-24 01:58:11]

Sorry, needed to stick with the regex to handle the potential for quotes...
using split here seemed like it would be more complex.  Let me know if you'd
rather see this as a split instead.

[Glen Murphy, 2009-03-24 21:09:40]

Drive-by nittery brought on by name confusion.

http://codereview.chromium.org/50039/diff/1045/1055
File chrome/installer/setup/eula/oem.js (right):

http://codereview.chromium.org/50039/diff/1045/1055#newcode5
Line 5: document.getElementById('ifr').src = results[1];
This indent should be two spaces.

[Glenn Wilson, 2009-03-24 21:15:35]

On 2009/03/24 21:09:40, Glen Murphy wrote:
> Drive-by nittery brought on by name confusion.
> 
> http://codereview.chromium.org/50039/diff/1045/1055
> File chrome/installer/setup/eula/oem.js (right):
> 
> http://codereview.chromium.org/50039/diff/1045/1055#newcode5
> Line 5: document.getElementById('ifr').src = results[1];
> This indent should be two spaces.

Fixed.  I had a tab in there that made it look like 4 spaces in my IDE.

[Mark Larson, 2009-03-24 21:17:04]

LGTmal